Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory Leaks in property setters #64

Open
robertjpayne opened this issue Mar 6, 2020 · 5 comments
Open

Memory Leaks in property setters #64

robertjpayne opened this issue Mar 6, 2020 · 5 comments

Comments

@robertjpayne
Copy link

I believe there is a memory leak with the below code:

pointer.assumingMemoryBound(to: self).initialize(to: value)

I think what's happening is if the value is not initialised it will be fine and no leak will occur (construction) but if you're replacing an existing value the old value is nullified without being free'd since swift loses track of it.

@robertjpayne
Copy link
Author

robertjpayne commented Mar 6, 2020

Ok so here is an easy to reproduce example below. You will notice the START inner value that is replaced never gets deallocated

import Runtime



class MySubObject {
    let label: String
    init(label: String) {
        self.label = label
    }
    
    deinit {
        print("MySubObject(label: \(self.label)) deinit")
    }
}

class MyObject {
    let inner: MySubObject
    
    init(label: String) {
        self.inner = MySubObject(label: label)
    }
    
    deinit {
        print("MyObject(inner.label: \(self.inner.label)) deinit")
    }
}

var obj: MyObject = MyObject(label: "START")


var newInner = MySubObject(label: "UPDATE")

let property = try Runtime.typeInfo(of: MyObject.self).property(named: "inner")
try property.set(value: newInner, on: &obj)

obj = MyObject(label: "FINALISE")

@robertjpayne
Copy link
Author

@wickwirew would be interested if you have any thoughts on this -- it's a really tricky thing and I'm not sure there is actually any feasible solution since we can't use Unmanaged<X> or anything to try and munge the ref counting and sorts.

@wickwirew
Copy link
Owner

At first glance, we may just need to make a call to swift_release manually for any heap values.

@robertjpayne
Copy link
Author

We ended up having to remove runtime entirely and use sourcery for our magic "reloading". Unfortunately didn't seem like there was a good fix that we felt was safe enough.

@blindmonkey
Copy link

We just ran into this in our project. Are there any plans to fix this? We started using Runtime pretty heavily, so I'm a bit concerned that these leaks will add up. We're thinking about using Unmanaged to work around this, but are a bit concerned that the fix will work against us if it's fixed in the library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants