Handle the FinalizationRegistry's context in HostEnqueueFinalizationRegistryCleanupJob
#111
+25
−22
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
…RegistryCleanupJob` Each FinalizationRegistry object has a context associated with it, which was the context at its construction. This context is used for calling its callback when any registered object is GC'd, which takes place in the `CleanupFinalizationRegistry` AO. In the current spec, `CleanupFinalizationRegistry` handles the swapping of this context, and restores it back at the end. This, however, causes a problem for hosts that report uncaught JS exceptions by running JS code (such as the `"error"` event on `window` in the web, or the `"unhandledException"` event on `process` in Node.js), since they should run this event in the FinalizationRegistry's creation context, but `CleanupFinalizationRegistry` exits that context before the end of the function. Since `CleanupFinalizationRegistry` (and `HostEnqueueFinalizationRegistryCleanupJob`) only clean up a single FinalizationRegitry, however, the context swapping can be pushed to the host hook instead, with the step about host-defined error reporting being inside that context. This ensures that such error reporting events are fired in the right context.
legendecas
approved these changes
Jan 23, 2025
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.
Each FinalizationRegistry object has a context associated with it, which was the context at its construction. This context is used for calling its callback when any registered object is GC'd, which takes place in the
CleanupFinalizationRegistryAO.In the current spec,
CleanupFinalizationRegistryhandles the swapping of this context, and restores it back at the end. This, however, causes a problem for hosts that report uncaught JS exceptions by running JS code (such as the"error"event onwindowin the web, or the"unhandledException"event onprocessin Node.js), since they should run this event in the FinalizationRegistry's creation context, butCleanupFinalizationRegistryexits that context before the end of the function.Since
CleanupFinalizationRegistry(andHostEnqueueFinalizationRegistryCleanupJob) only clean up a single FinalizationRegitry, however, the context swapping can be pushed to the host hook instead, with the step about host-defined error reporting being inside that context. This ensures that such error reporting events are fired in the right context.