Env-cleanup hooks can miss a descriptor mid-close/mid-reopen (registry reachability + dangling entry reference)

[kriszyp]

Follow-up from the #694 cross-model review (adjudicated significant-narrow; shared with the pre-existing listener cleanup path, not introduced by #694).

The module env-cleanup hooks (DBRegistry::RemoveListenersByEnv, DBRegistry::ReleaseCommitCompletionsByEnv) find shared descriptors by walking instance->databases[*].descriptor. Two windows make a live descriptor unreachable from that map:

1. OpenDB's wait predicate does entry.descriptor.reset() when it observes a closing descriptor (db_registry.cpp), dropping the registry's ref while the closer is still inside finishClose(). A worker env tearing down in that window is not scrubbed from the descriptor; the backstop is finishClose()'s own release pass (which runs after the commit thread is joined), but that pass can then call napi_release_threadsafe_function on a tsfn whose creating env has concurrently finished teardown — a narrow UAF.
2. Related pre-existing hazard in the same protocol: OpenDB's waiting thread holds auto& entry = entryIterator->second (a reference into the map node) across the condition wait, while the closer's guarded erase can erase that node (the guard erases when !eraseIt->second.descriptor, which is exactly the state OpenDB's predicate creates). The woken waiter then evaluates its predicate against freed node storage.

Candidate categorical fix: give the registry a secondary set of all live descriptors (weak_ptr), maintained under databasesMutex, that the env-cleanup hooks walk instead of the entry map — descriptor reachability then no longer depends on entry-map lifecycle. The entry dangling reference wants a re-find-after-wake (or shared_ptr'd entry values) in OpenDB.

#694 already narrowed the surface: DestroyDB now closes before unlinking (CloseDB's discipline).

🤖 Generated with Claude Code

[kriszyp]

Reproduced this deterministically while triaging a Windows Bun CI failure on #742 (db.setCompression()) — the CI job failed with Worker exited unexpectedly right after the entire Vitest suite (all 722 tests) passed, i.e. during the single Bun process's final teardown, with no test-level failure. That symptom (crash after everything already passed) pointed at a process-lifecycle issue rather than anything in #742's diff.

Repro: rebuilt the native binding with ASan (ROCKSDB_ASAN=1 node-gyp rebuild, per AGENTS.md) and ran under Bun on Linux (LD_PRELOAD=libasan.so bun --bun vitest run test/compression.test.ts). Deterministic crash, same spot every time (3/3 runs):

AddressSanitizer: CHECK failed: asan_poisoning.cpp:85 "((AddrIsAlignedByGranularity(addr))) != (0)" (0x0, 0x0)
    #4 __asan_handle_no_return
    #5 rocksdb_js::DBRegistry::OpenDB(...) db_registry.cpp:376
    #6 rocksdb_js::DBHandle::open(...) db_handle.cpp:332
    #7 rocksdb_js::Database::Open(...) database.cpp:1534

Line 376 is entry.descriptor = DBDescriptor::open(path, options); in the "not already open" branch of OpenDB — opening a brand-new path right after a previous test's db.close(), no setCompression() involved anywhere in the crashing run.

Confirmed pre-existing, not introduced by #742: same rebuild-with-ASan repro on main (ea83ff46, the commit #742 branched from) crashes identically, same line, same test boundary. So this isn't specific to the compression work — it's a latent race in the open/close path that #742's larger test file (more DB churn per Vitest process, since fileParallelism:false runs the whole suite in one Bun process) apparently made likelier to hit in CI.

3 plain (non-ASan) local runs of the full suite under Bun on Linux didn't reproduce it, so it's low-probability without ASan's slower/instrumented execution — consistent with a rare timing window, and with Windows CI runners (slower, more contended) hitting it more often than local Linux.

Given this issue's write-up already calls out OpenDB's dangling-reference/UAF hazard around entry and the registry's guarded erase during a concurrent close, and the crash trace lands exactly inside OpenDB's "not already open" branch right after a prior test's close, this looks like the same hazard class (open-after-close on a registry entry mid-teardown), possibly a plain single-threaded instance of it (no worker_threads needed — just back-to-back open()/close()/open() on the main thread). Sharing in case the ASan trace narrows down the categorical fix.

For #742 itself: not fixing this here (needs the categorical fix already scoped in this issue, and touching OpenDB's teardown/registry logic under time pressure inside an unrelated feature PR is exactly the kind of change AGENTS.md warns against making casually). Added extra CI retries for the Windows Bun job as a stopgap so this doesn't block unrelated PRs — see the workflow diff on #742.