Fixup GC'ing objects with callbacks intermittent crasher #208
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jhugman
commented
Jan 23, 2025
| @@ -414,7 +414,7 @@ pub(crate) impl FfiCallbackFunction { | |||
| } | |||
|
|
|||
| fn is_blocking(&self) -> bool { | |||
| self.name() != "RustFutureContinuationCallback" | |||
| !self.is_free_callback() && self.name() != "RustFutureContinuationCallback" | |||
There was a problem hiding this comment.
As suspected, it's a one line fix.
jhugman
added a commit
that referenced
this pull request
Jan 23, 2025
This is a followup to #208 to tidy up the `FfiCallbackFuncrtion` extensions in `extensions.rs`. Most notably, it generalizes the `is_blocking` function to callback functions that do not have a return and are not expected to error, ever.
Merged
jhugman
added a commit
that referenced
this pull request
Jan 30, 2025
It's time for a release. If your change hasn't landed before this release, don't worry, releases are cheap! We can do another one! # 0.28.3-2 ## ✨ What's New * Add `--profile` build argument ([#192](#192)) * Thank you [@Johennes](https://github.com/Johennes)! ## 🦊 What's Changed * Adjust template to allow for hot reload via metro of running apps ([#207](#207)). * Stabilise `require.resolve` by looking up `package.json` instead of entrypoint ([#200](#200)). * Thank you [@hassankhan](https://github.com/hassankhan)! * Split compat job by platform and version ([#211](#211)). * This shows on the README.md if builder-bob or React Native has changed breaking the tutorial. * Thank you [@Johennes](https://github.com/Johennes)! * Fixed GC'ing objects with callbacks intermittent crasher ([#208](#208) and [#209](#209)) * Reproducibly pick the same library file when using `--and-generate` ([#194](#194)) * Thank you [@Johennes](https://github.com/Johennes)! ## 🌏🕸️ WASM! * Fixtures `coverall`, `custom-types-example`, `enum-types`, `trait-methods` ([#202](#202)). * Switched from passing `ArrayBuffer`s to using `Uint8Array`, to accommodate WASM better. ([#187](#187)) Callbacks now have UniffiResult to communicate between typescript and C++ ([#205](#205)). * Fixtures `coverall2` and `rondpoint` ([#191](#191)). * Fixture `arithmetic` ([#188](#188)). ## 📰 Documentation * Remove duplicate parentheses ([#203](#203)). * Minor typo fixes in GC docs ([#204](#204)). * Remove reference to name field in the ubrn.config.yaml docs ([#189](#189)). **Full Changelog**: 0.28.3-1...0.28.3-2
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
According to The Big O of Code Reviews, this is a O(n) change.
Fixes zzorba/ubrn-sample-app#1
The investigation for this was in zzorba/ubrn-sample-app#1.
The key insight here is that the "fake GC" destructor is calling into Rust which then calls a free callback in hermes, which somehow triggers another GC.
The fix is to schedule the free callback on the event loop.
This also fixes previously encountered
"Stats not submitted."assertion failures.Possible other things to do for this: document that you shouldn't call callbacks in a
Drop::dropthough I'm not sure how to phrase that without being too restrictive.