Draft: Hooking Hermes GC to Rust (do not merge) #10
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
This changes the implementation of `bless` to a HostObject. Ultimately, this commit may not make it to main, as it doesn't yet work.
|
This is leftover work from #9. It is not ready to land: it should not need to go through the JS, but should likely go straight to Rust from the C++ destructor. |
|
Superceded by #46 . |
jhugman
added a commit
that referenced
this pull request
Jul 31, 2024
Fixes #10 . According to [The Big O of Code Reviews](https://www.egorand.dev/the-big-o-of-code-reviews/), this is a O(_n_) change. This PR adds C++ destructor hooks from uniffi `interface` objects, to call the corresponding Rust `drop` methods. The impedance mismatch between a GC'd language and an [RAII](https://en.wikipedia.org/wiki/Resource_acquisition_is_initialization) language means that one is a leaky abstraction for the other. From facebook/hermes#982 (comment): > Generally speaking, when dealing with native resources, it is recommended to free the native resource explicitly when you are done with it, because garbage collection is unpredictable and should not be relied on for freeing native resources timely." From the HostObject's destructor docs: > The C++ object's dtor will be called when the GC finalizes this object. (This may be as late as when the Runtime is shut down.) You have no control over which thread it is called on. This will be called from inside the GC… For this reason, the `uniffiDestroy` method is left in place for now: if you absolutely need to drop a Rust peer object, then you should call the `uniffiDestroy` method of the correspond JS object. From the integration tests alone, it is difficult to show that this is working; they are too short-lived for the Runtime to be still running and garbage collection to have taken place. However, the `test_callbacks.ts` did cause an ungraceful shutdown when the destructor called into Rust which then called back into JS– which had already been killed. The situation will only really get better when React Native/Hermes [supports a robust `FinalizationRegistry` implementation](facebook/hermes#1440).
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.
This changes the implementation of
blessto a HostObject.Ultimately, this commit may not make it to main, as it doesn't yet work.