feat: Guaranteed finalizers #1076
Merged
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 is a way to fix #1066, which was blocking denoland/deno#15760, but it has not been discussed with the core team and might not be a good idea. I'm raising this PR as a way to initiate the discussion. |
Currently, when a finalizer callback is registered, it is not guaranteed to be called if there is a global reference to the corresponding object that survives the isolate. This is because the finalizer callback takes a `&mut Isolate`, and so it must be called before the isolate is fully destroyed, but all existing globals (including possibly the one being currently finalized) are still usable while there still exists a mutable reference to the isolate. However, there are still use cases for having finalizers that are guaranteed to run regardless of any remaining globals, but that don't require any interaction with the isolate. This change adds them. This change also changes the context annex to use a guaranteed finalizer, fixing a bug with context slots not being freed if there were any globals to the context at the time the isolate is dropped. Closes denoland#1066.
andreubotella
force-pushed
the
guaranteed-finalizer
branch
from
c460930 to
7c7dd19
Compare
|
I added tests for the guaranteed finalization API. PTAL |
bartlomieju
approved these changes
Oct 6, 2022
bartlomieju
changed the title
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.
Currently, when a finalizer callback is registered, it is not guaranteed to be called if there is a global reference to the corresponding object that survives the isolate. This is because the finalizer callback takes a
&mut Isolate, and so it must be called before the isolate is fully destroyed, but all existing globals (including possibly the one being currently finalized) are still usable while there still exists a mutable reference to the isolate.However, there are still use cases for having finalizers that are guaranteed to run regardless of any remaining globals, but that don't require any interaction with the isolate. This change adds them.
This change also changes the context annex to use a guaranteed finalizer, fixing a bug with context slots not being freed if there were any globals to the context at the time the isolate is dropped.
Closes #1066.