Skip to content

Pin the native module in memory so worker thread exit cannot unload it - #235

Merged
penberg merged 1 commit into
mainfrom
ci-windows-diag
Sep 18, 2026
Merged

penberg merged 1 commit into
mainfrom
ci-windows-diag

Conversation

@penberg

@penberg penberg commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Problem

The Test bindings on x86_64-pc-windows-msvc - node@22 job has failed on roughly half of all main runs since 2026-05-30, always the same way: every sync test passes, ava's summary never prints, and the step ends with a bare exit code 1.

exit code 1 is misleading. GitHub's pwsh shell reports every abnormal exit that way, including process.abort() and a 0xC0000005 access 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::~Environment closes 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 Windows FreeLibrary() unmaps the DLL underneath them and they fault.

Running the suite under cdb caught 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, cdb no 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::drop calls local::Connection::disconnect(), then the inner local::Connection field's own Drop calls disconnect() again on the same raw handle, so sqlite3_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 in functionDestroy. It only reproduces with the debug heap disabled (_NO_DEBUG_HEAP=1), which is why it hid from the debugger at first. Symbolized stack:

napi ObjectFinalize -> libsql_js::Database::drop (lib.rs:245)
-> Arc<libsql::Connection>::drop_slow
-> libsql::local::Connection::drop -> disconnect() (local/connection.rs:110)
-> sqlite3_close_v2 -> functionDestroy -> access violation

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 libsql main. Until that is fixed and bumped here, expect the Windows job to still fail about 1 run in 25.

@penberg penberg changed the title [ci diag] Diagnose intermittent Windows node@22 test failure (do not merge) Pin the native module in memory so worker thread exit cannot unload it Sep 15, 2026
@penberg
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
penberg merged commit b2b7691 into main Sep 18, 2026
25 of 26 checks passed
@penberg
penberg deleted the ci-windows-diag branch September 18, 2026 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant