Repository navigation
fix(napi): don't panic when the instance cache drops at process exit - #43
Merged
Merged
Conversation
With wrappers still cached, process.exit() ran the instance cache's
thread-local destructor after REF_GRAVEYARD's, and the entry's Drop
panicked on the destroyed thread-local ("cannot access a Thread Local
Storage value during or after destruction"). The entry drop and
evict_instance now skip a thread-local that is already gone; the
deferred napi_ref has nothing left to clean up at that point.
Repro: CryptographicBuffer.DecodeFromHexString('0a0b'); process.exit(0)
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
This file contains hidden or 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
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.
On Node,
process.exit()with WinRT wrappers still in the napi instance cache panics during shutdown:NAPI_INSTANCE_CACHE's thread-local destructor drops its entries afterREF_GRAVEYARDhas already been destroyed, andInstanceCacheEntry::droppushes into the graveyard with.with(). Both that push andevict_instancenow usetry_withand skip a destroyed thread-local. Nothing is lost: at that point the process is exiting and the deferrednapi_refcan't be deleted anyway.The repro exits cleanly on a build from before #40 and panics on current
main, so it became reachable with #40–#42.Repro (windows-napi on Node):
Testing
main, exits cleanly with this change (3/3 runs).npm test: 442/442.