fix(usb): stop finalizer from dereferencing tsfn-owned callbackRef during shutdown (#790)

[cb1kenobi]

Fixes #790.

Problem

Sending ctrl-c to quit Harper crashed during shutdown (SIGSEGV) on macOS. The faulting frame:

napi_get_reference_value                       <- EXC_BAD_ACCESS
rocksdb_js::userSharedBufferFinalize
... BufferFinalizer::FinalizeBufferCallback
... Environment::RunCleanup / FreeEnvironment  <- worker/process teardown

getUserSharedBuffer(key, buf, { callback }) registers a listener whose napi_ref ownership is transferred to the listener's threadsafe function in addListener (the tsfn finalizer napi_delete_references it). The buffer's UserSharedBufferFinalizeData also stashed that same raw ref as a borrowed pointer. At shutdown the listener is torn down (removeListenersByOwner on db.close(), or removeListenersByEnv on env teardown) → the tsfn finalizer deletes the ref → then userSharedBufferFinalize calls napi_get_reference_value on the freed ref: a use-after-free.

On a plain build the freed slot usually reads back benign, so it only bit intermittently in production. It needs no worker or GC — a plain main-thread db.close() + natural process exit reproduces it, because the ref lifetime belongs to the tsfn, not the finalizer.

Fix

Remove the listener by identity instead of re-resolving a ref the finalizer doesn't own:

• EventEmitter::addListener now returns the std::shared_ptr<ListenerCallback> it already allocates (was napi_ref).
• UserSharedBufferFinalizeData holds a std::weak_ptr<ListenerCallback> instead of the raw napi_ref.
• New env-free overload EventEmitter::removeListener(key, target) releases the tsfn (which queues the ref deletion on the JS thread, as before) and erases the map entry — without touching the napi_ref.

The finalizer locks the weak_ptr and, only if the listener is still registered, removes it by identity. If it was already torn down (db.close() runs removeListenersByOwner before descriptor.reset()), the lock fails and there's nothing to do. The map holds the sole strong ref to the ListenerCallback, and close/env-teardown/finalizer all run on the same env thread, so there's no cross-thread lifetime race.

The buffer.cancel() / db.removeListener(key, fn) path (removal by function value) and the "cleanup callbacks on GC" behavior are unchanged.

Testing

• New macOS-only, Guard-Malloc-gated child-process regression test (test/user-shared-buffer-shutdown.test.ts + fixture): opens a DB, gets user-shared buffers with callbacks, db.close(), keeps them referenced, and exits naturally so RunCleanup finalizes them. SIGSEGVs before this change, passes after (verified 20+ runs under Guard Malloc across three repro variants). Gated to macOS because a plain build reads benign freed memory and can't fault deterministically.
• All existing user-shared-buffer, notify, events, global-events, notify-teardown, and shutdown suites pass. pnpm check (type-check, lint, format) is clean.

Reviews

Deep review (two independent passes) and cross-model review (Codex + Gemini): no crash-safety, concurrency, lifetime, or security blockers. Follow-ups addressed here: repro DB dirs are now cleaned up (honoring KEEP_FILES), and an overbroad finalizer comment was tightened. Two Gemini "majors" were false positives (the store.ts layer unwraps options.callback before native; .mts type-stripping is unflagged at the repo's engines floor and existing fixtures spawn identically — the test runs green).

Pre-existing follow-ups (out of scope, noted for tracking)

• getUserSharedBuffer allocates finalizeData before napi_create_external_arraybuffer; on that (rare) failure it leaks. RAII cleanup would tidy it.
• If defaultBuffer isn't an ArrayBuffer, the listener is registered before the validation throw, so it lingers until close().

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request resolves a shutdown use-after-free (UAF) vulnerability on macOS related to user-shared buffers by replacing raw napi_ref handles with std::shared_ptr<ListenerCallback> and std::weak_ptr<ListenerCallback> references. This allows the finalizer to safely remove listeners by identity without dereferencing deleted references. Additionally, a regression test suite has been introduced to verify the fix. The review feedback suggests improving the implementation by using std::find instead of a manual loop in EventEmitter::removeListener and enforcing const-correctness by passing the key parameter as const std::string& in DBDescriptor::removeListener.

[github-actions]

📊 Benchmark Results

get-sync.bench.ts

getSync() > random keys - small key size (100 records)

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 lmdb	1	23.45K ops/sec	42.65	41.17	2,120.309	0.143	117,245
🥈 rocksdb	2	11.19K ops/sec	89.38	86.10	31,480.436	1.23	55,943

getSync() > sequential keys - small key size (100 records)

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 lmdb	1	28.46K ops/sec	35.14	34.07	549.423	0.103	142,282
🥈 rocksdb	2	11.35K ops/sec	88.14	84.77	2,570.684	0.114	56,730

ranges.bench.ts

getRange() > small range (100 records, 50 range)

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 lmdb	1	24.79K ops/sec	40.34	35.98	1,887.227	0.283	123,942
🥈 rocksdb	2	15.86K ops/sec	63.06	55.60	1,090.169	0.121	79,292

realistic-load.bench.ts

Realistic write load with workers > write variable records with transaction log

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	381.90 ops/sec	2,618.506	161.281	51,311.315	13.69	764
🥈 lmdb	2	26.23 ops/sec	38,124.036	349.524	1,224,014.9	137.008	64.00

transaction-log.bench.ts

Transaction log > read 100 iterators while write log with 100 byte records

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	39.57K ops/sec	25.27	11.22	13,973.031	0.601	197,867
🥈 lmdb	2	446.42 ops/sec	2,240.02	199.096	13,455.135	1.28	2,233

Transaction log > read one entry from random position from log with 1000 100 byte records

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	746.75K ops/sec	1.34	1.17	4,664.704	0.194	3,733,745
🥈 lmdb	2	465.91K ops/sec	2.15	1.18	17,778.646	0.747	2,329,534

worker-put-sync.bench.ts

putSync() > random keys - small key size (100 records, 10 workers)

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	815.46 ops/sec	1,226.296	1,069.783	2,141.561	0.337	1,631
🥈 lmdb	2	1.17 ops/sec	853,049.574	819,134.245	879,796.305	1.83	10.00

worker-transaction-log.bench.ts

Transaction log with workers > write log with 100 byte records

Implementation	Rank	Operations/sec	Mean (ms)	Min (ms)	Max (ms)	RME (%)	Samples
🥇 rocksdb	1	22.66K ops/sec	44.14	30.77	556.251	0.580	45,316
🥈 lmdb	2	816.81 ops/sec	1,224.279	185.834	16,555.638	5.26	1,634

---

Results from commit 9d268f8

[kriszyp]

Great!