Repository navigation
Fix Windows crash on exit: upgrade napi to 3.12.7 - #8
Merged
Merged
Conversation
napi 3.2.2 shuts the tokio runtime down from a process-exit destructor (#[ctor::dtor]). On Windows, ExitProcess has already terminated the runtime's threads by then, so the shutdown touches freed state and the process dies with 0xC0000005. napi 3.6.0 moved the shutdown to a Node env cleanup hook, which runs while the threads are still alive (napi-rs#3026). Measured on windows-latest, 40 fresh processes per case, napi 3.2.2 vs 3.12.7: Electron 32 app, load + call + quit: 40/40 crash vs 0/40 Node worker thread, load + call: 11-31/40 vs 0/40 ava test suite: 8-9/40 vs 0/40 Node main thread, load + call: 0/40 vs 0/40 The Electron case is how Moss loads this addon, so Moss on Windows is expected to crash on every quit with any release built on napi 3.2.2.
Runs a load-and-call probe 10 times in fresh processes, on the main thread and in a worker thread, and asserts every exit code is 0. With napi 3.2.2 the worker probe crashed on Windows in 11-31 of 40 runs, so 10 runs catch a regression of that kind with near certainty; with 3.12.7 it crashed in none. Runs in CI on every binding.
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
1 participant
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.
Problem
On Windows, any process that has loaded this addon can crash as it exits, with
0xC0000005(access violation). In an Electron 32 app, which is how Moss loads it, this happened on every quit. It showed up as an intermittent failure of the Windows Node 20 test job on thev0.700.0tag run: all tests passed, then the process crashed on exit. The previous blank test never loaded the binding, so CI never saw it before.Cause
napi 3.2.2 stops its tokio runtime from a process-exit destructor (
#[ctor::dtor] fn thread_cleanup→shutdown_async_runtime()). On Windows,ExitProcesshas already terminated the runtime's threads when that destructor runs, so the shutdown touches freed state. napi 3.6.0 moved native shutdown to a Node env cleanup hook, which runs while the threads are still alive (napi-rs/napi-rs#3026, "shutdown runtime at env cleanup on windows").Measurement
On
windows-latest, each case ran 40 times, each in a fresh process. The count is exits with0xC0000005:Every
we-rust-utilsrelease so far is built on napi 3.2.2, including0.601.xand0.700.0.Changes
index.js/index.d.tsare unchanged, and the hash regression tests pass.__test__/exit.spec.mjsruns a load-and-call probe 10 times in fresh processes, on the main thread and in a worker thread, and fails on any non-zero exit code. It runs in CI on every binding. At the worker crash rate measured on 3.2.2, 10 runs catch that kind of regression with near certainty.After merge
Release
0.700.1the same way as0.700.0, then bump Mossmain-0.7to^0.700.1.Not in this PR
main-0.6. The 0.6 line has the same bug.