search

wickwirew / Runtime

A Swift Runtime library for viewing type info, and the dynamic getting and setting of properties.
MIT License
1.08k stars 94 forks source link

Fix memory leak with setters #76

Closed blindmonkey closed 3 years ago

blindmonkey commented 3 years ago

This PR aims to fix issue #64, which reports a memory leak when setting a field which stores a reference type.

Unfortunately, I'm not very familiar with raw memory manipulation, so I'm not 100% sure whether this is the correct thing to do in all cases, but it appears to address the issue when I apply this fix in our project.

I wasn't able to run the tests locally, but I'm happy to look into any tests this PR breaks.

wickwirew commented 3 years ago

Thanks for looking into this! Glad it was that simple of a fix. Ran everything locally an confirmed the leak is gone. IIRC earlier in Swift doing pointee = value would leak, but initialize(to: value) did not. I'm not sure what changed, or maybe I mixed them up.

This does break a few tests though. When using pointee, it requires that the memory already be initialized. So in the case for constructing a value from the type, that fails. Maybe on the set method you can add initializing: Bool and conditionally use initialize(to:) and pointee. Then set that to true when being used to construct the value in Factory.swift

blindmonkey commented 3 years ago

Thanks for the feedback, @wickwirew! I added an option to set called initialize, with a default value of false. I was able to get the tests to run (hint: comment out LinuxMain.swift), so I was able to confirm that everything passes.

wickwirew commented 3 years ago

Awesome, thanks again! Pushed a new version 2.2.2