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

Fix block2 memory management #568

Merged
merged 2 commits into from
Jan 24, 2024
Merged

Fix block2 memory management #568

merged 2 commits into from
Jan 24, 2024

Conversation

madsmtm
Copy link
Owner

@madsmtm madsmtm commented Jan 22, 2024

Part of #168. Fixes upstream SSheldon/rust-block#9.

The way block2 currently works is that you create the StackBlock, and then you must call copy to convert it to an RcBlock. This is very error-prone, especially since you can't ensure that Objective-C doesn't call _Block_copy when you pass your stack block to an external function, so forgetting to call copy usually ends up causing double-frees.

Instead, we should implement StackBlock to use Clone once _Block_copy is called, and for the cases where that's impossible, the user should construct the RcBlock directly.

Clang notes: In ARC-enabled Clang, it is impossible to get the equivalent of StackBlock, since it effectively always does a .to_rc() on creation. Non-ARC Clang puts the block on the stack, and you're expected to call Block_copy yourself.

@madsmtm madsmtm added bug Something isn't working A-block2 Affects the `block2` crate I-unsound A soundness hole labels Jan 22, 2024
@madsmtm madsmtm marked this pull request as draft January 22, 2024 15:36
@madsmtm madsmtm mentioned this pull request Jan 22, 2024
11 tasks
@madsmtm madsmtm marked this pull request as ready for review January 23, 2024 20:53
@madsmtm madsmtm force-pushed the fix-block-memory-management branch 8 times, most recently from 6c290ec to 46a70d7 Compare January 23, 2024 23:24
@madsmtm madsmtm force-pushed the fix-block-memory-management branch from 46a70d7 to 216efbb Compare January 24, 2024 19:40
@madsmtm madsmtm merged commit a6389c3 into master Jan 24, 2024
19 checks passed
@madsmtm madsmtm deleted the fix-block-memory-management branch January 24, 2024 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-block2 Affects the `block2` crate bug Something isn't working I-unsound A soundness hole
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant