Pin the native module in memory so worker thread exit cannot unload it - #235
Merged
Merged
Conversation
penberg
force-pushed
the
ci-windows-diag
branch
from
September 15, 2026 20:44
1a5eeab to
decceb1
Compare
penberg
marked this pull request as ready for review
September 15, 2026 20:44
Node.js unloads a native addon when the worker thread that loaded it exits (Environment::~Environment closes every addon a non-main thread loaded), unless another environment still holds the library open. This addon owns process-wide threads that outlive any single environment: the tokio runtime behind the synchronous API and the query timeout thread. On Windows, FreeLibrary() unmaps the DLL underneath them and the next instruction they execute faults with an access violation. This is most of the intermittent 'Test bindings on x86_64-pc-windows-msvc - node@22' failure: ava runs each test file in a worker thread, and the crash showed up as a bare exit code 1 because pwsh reports every abnormal exit that way. Running the suite under cdb caught the faulting thread at <Unloaded_libsql.win32-x64-msvc.node>, and pinning drops the failure rate from about 20% to about 4% of runs. Pin the library on first use of either process-wide thread: GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_PIN) on Windows and a RTLD_NODELETE dlopen() of our own image elsewhere. The remaining 4% is a double sqlite3_close_v2() in libsql itself: LibsqlConnection::drop and local::Connection::drop both call disconnect() on the same handle, and the second call reads the freed sqlite3 struct. Verified by vendoring libsql with an idempotent disconnect(): 0 crashes in 100 runs. That fix belongs upstream.
penberg
force-pushed
the
ci-windows-diag
branch
from
September 15, 2026 20:45
decceb1 to
a67b20a
Compare
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.
Problem
The
Test bindings on x86_64-pc-windows-msvc - node@22job has failed on roughly half of allmainruns since 2026-05-30, always the same way: every sync test passes, ava's summary never prints, and the step ends with a bareexit code 1.exit code 1is misleading. GitHub's pwsh shell reports every abnormal exit that way, includingprocess.abort()and a0xC0000005access violation (verified with probe steps). The real exit code was-1073741819, an access violation in the ava process.Root cause 1 (this PR): Node unloads the addon while our threads still run in it
Node unloads native addons that were loaded from a worker thread when that worker exits (
Environment::~Environmentcloses them, see nodejs/node#37024). ava runs each test file in a worker thread, and this addon owns process-wide threads that outlive any single environment: the tokio runtime behind the sync API and the query timeout thread. On WindowsFreeLibrary()unmaps the DLL underneath them and they fault.Running the suite under
cdbcaught the faulting thread at<Unloaded_libsql.win32-x64-msvc.node>+0x6a008e.Fix: pin the library in memory on first use of either process-wide thread (
GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_PIN)on Windows,dlopen(RTLD_NODELETE)of our own image on unix). With this,cdbno longer sees any module unload event, and the plain failure rate drops from ~20% to ~4% of runs.Root cause 2 (upstream, not in this PR): libsql closes every connection twice
The remaining ~4% is a use-after-free inside libsql.
LibsqlConnection::dropcallslocal::Connection::disconnect(), then the innerlocal::Connectionfield's ownDropcallsdisconnect()again on the same raw handle, sosqlite3_close_v2()runs twice. The second call usually returns misuse harmlessly, but when another thread reuses the freed block first, sqlite walks garbage and faults infunctionDestroy. It only reproduces with the debug heap disabled (_NO_DEBUG_HEAP=1), which is why it hid from the debugger at first. Symbolized stack:Verified by vendoring libsql with an idempotent
disconnect()(null the raw pointer after closing): 0 crashes in 100 runs, versus 4/100 without it. The same double drop is present on libsqlmain. Until that is fixed and bumped here, expect the Windows job to still fail about 1 run in 25.