Brendonovich / swift-rs

Call Swift functions from Rust with ease!
Apache License 2.0
253 stars 30 forks source link

Decrement ARC counts on SRObject drop #2

Closed Brendonovich closed 2 years ago

Brendonovich commented 2 years ago

After looking over swift-bindgen's code, it seems that it's possible to manually alter ARC counts for arbitrary pointers, meaning that it should be possible to let the swift runtime know when rust is done with an SRObject by decrementing the corresponding ARC. This should avoid memory leaks that I believe exist at the moment, since I don't thing swift knows when rust is finished with a value.

Brendonovich commented 2 years ago

Did some testing and it doesn't seem like this is possible

0rvar commented 2 years ago

@Brendonovich did you figure out how to release Swift memory from Rust?

Brendonovich commented 2 years ago

@0rvar No, I wasn't able to figure it out. I was able to call the release and retain functions, but often the retain count of objects would be in the thousands and I had no idea why. No matter what I tried, dropping an SRObject wouldn't trigger the corresponding Swift object's deinit(). Would be great to figure it out but I'm not sure I want to try again haha.

0rvar commented 2 years ago

@Brendonovich that's too bad. Played around with swift_release and other free functions, but I still leaked like a sieve. I'll rewrite the swift code in rust using objc2 instead.

Brendonovich commented 2 years ago

@0rvar My curiosity got the better of me and I tried a different approach for doing retains and releases: Instead of doing so via those Rust bindings, I defined functions on the Swift side that use Unmanaged.passUnretained to retain and release objects at will, and to my surprise it works! I'm able to have this swift binding:

@_cdecl("decrease_retain_count")
public func decreaseRetainCount(obj: UnsafePointer<AnyObject>) {
    let _ = Unmanaged.passUnretained(obj.pointee).release();
}

and call it like this:

extern "C" {
    fn decrease_retain_count(obj: &SRString);
}

impl Drop for SRString {
    fn drop(&mut self) {
        unsafe { decrease_retain_count(&self) }

        println!("Dropped SRString");
    }
}

Works just as I'd expect, printing the Drop message as well as the deinit() message! Knowing this I may just do a bit of a rewrite and give this library some much needed attention.

0rvar commented 2 years ago

@Brendonovich I've upgraded to latest master, but still getting memory leaking. I'll create a branch reproducing the leaks I get.

EDIT: Leaking loads when running examples in this branch: https://github.com/0rvar/swift-rs/tree/orvar/memory-leaks However, it seems deinit is run in Swift? Not sure what's going on.

Brendonovich commented 2 years ago

@0rvar That's interesting. I haven't actually been measuring for memory leaks only checking that objects are being released properly. Building your example without the final loop, navigating to the executable's folder and running leaks -atExit -- ./example results in the output 0 leaks for 0 total leaked bytes. What are you using to measure the leaks?

0rvar commented 2 years ago

@Brendonovich I'm just running the program with the loop and looking at memory usage in Activity Monitor

Brendonovich commented 2 years ago

@0rvar Hmm, Instruments doesn't report anything wrong for some reason. I did the loop for 30000 iterations and then paused to allow for swift to potentially deallocate stuff but nope it just stays there. No idea why this would be happening

image
Brendonovich commented 2 years ago

Interestingly, if the function called in the loop is made to just return a big SRString, then no continuous climb of memory usage occurs.

image

Additionally, in the get_mounts loop, Instruments reports that Volume, SRArray, etc are actually being destroyed:

image image

Hence, I don't think this is a problem with the deallocation, but rather with how the process of fetching the mounts works - and maybe there's a memory leak within it.

And it turns out that yep, that was the problem. Calling mountedVolumeURLs was caching data, so wrapping it in an autoreleasepool fixed a lot of the leaks. Additionally, the for loop that iterates each url had to be wrapped in an autoreleasepool to allow swift to do cleanups between iterations, which fixed the last of the leaks. Now swift-rs definitely deallocates properly and I know more about writing Swift!