Fix memory leak for Option<SwiftType>
#273
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit fixes a memory leak when passing
Option<SwiftType>
toextern "Rust"
functions.For example, the following bridge module would lead to memory leaks:
When the above
rust_fn_takes_swift_type
function was called theSomeSwiftType
would be retained twice.When the Rust side later freed
SomeSwiftType
the retain count woulddrop from 2 to 1.
Since there was still 1 retain,
SomeSwiftType
would never get freed.Impact
Support for passing
Option<SomeSwiftType>
from Swift to Rust wasintroduced earlier today.
This functionality was released in
swift-bridge = "0.1.54"
.W will be deploying `swift-bridge = "0.1.55" today. "0.1.55" will
contain a fix for the leak.
Because support for
Option<SomeSwiftType>
was released today, and thefix will be released today, it should be unlikely that anyone has
experienced this leak.
We will yank "0.1.54" from "crates.io".
All users (probably zero) that are bridging
Option<SomeSwiftType>
shouldupgrade from "0.1.54" to "0.1.55" to.
Solution
This commit does the following:
When passing ownership of a Swift type from Swift to Rust we now
increment the retain count by one. Before this commit we were
incrementing it by two.
When passing ownership of a Swift type from Rust to Swift we avoid
calling the the Rust handle's
Drop
implementation. We avoid this byusing
std::mem::forget
.This ensures that the retain count does not get decremented.
When passing ownership of a Swift type from Rust to Swift the Swift
side does not increment the retain count.
With all of the above we should now be able to:
Create an
Option<SwiftType>
Swift type on the Swift sidePass ownership of the `Option to Rust. The retain count is now 1.
Pass ownership back to Swift. The retain count is still 1.
Before this commit, when passing an
Option<SwiftType>
it was:Create a Swift type on the Swift side
Pass
Option<SwiftType>
to Rust. Retain count is now 2Rust passes ownership back to Swift. Retain count is now 1
Retain count never hits 0. Memory has been leaked.