Reject in-flight async gets when an orphaned transaction is GC'd

[cb1kenobi]

Closes the GC-time UAF on an in-flight transactional async get that #768's orphan-release path newly made reachable: onWrapperCollected no longer calls close() while async work is live (the wait can time out, then delete this->txn races the worker). Pending gets are cancelled and reject with "Transaction is closed"; AsyncGetState pins the handle with a shared_ptr and each get complete callback re-enters onWrapperCollected so only the last in-flight get closes. In-flight commit is unchanged (still deferred to completeCommitWork).

Gemini's suggested delete state then close-if-count-0 is not used: unregisterAsyncWork already runs in execute via signalExecuteCompleted(), so the count is already decremented when complete runs. Re-entering onWrapperCollected reuses the existing count>0 gate.

Unresolved from the follow-up review (not blocking this Gemini thread): the two-get test locks both-reject + snapshot cleanup, not a >5s stalled sibling that would have timed out the old close(); the execute-delay seam remains on the production cache-miss get path (same as the first commit).

For the human reviewer

1. Reject vs complete a still-awaited read. Dropping the wrapper while holding only the promise now rejects (GC-timed) instead of resolving. Completing the read is memory-safe now that AsyncGetState holds a shared_ptr; rejection is the stated intent and what the tests lock in. Alternative: let execute finish Get and still close in complete. A "no" on rejection is a small behavior change for getBinary callers that don't retain the Transaction.

Verification

Extended test/transaction-orphan-gc.test.ts (should reject an in-flight async get when the transaction is dropped, should reject two in-flight async gets when the transaction is dropped). Ran node --expose-gc ./node_modules/vitest/vitest.mjs test/transaction-orphan-gc.test.ts — 7 passed. pnpm type-check clean via pre-commit.

Review coverage

Authored by Cursor Grok 4.6. Cross-model review of 2ee75bdd via prepush-review.mjs vs origin/kris/txn-registry-weakptr: gemini via agy (default model) ✓, gpt-5.6-sol (codex graded) ✓, Harper domain ✗ (claude not on PATH — findings unadjudicated), cursor-grok disabled as authoring family. Receipt @ 2ee75bd.

Complexity: complicated

_{Review-Coverage: authored=claude; ran=codex; declined=gemini,cursor-grok,cursor-composer,domain; rounds=2 @ 2ee75bd}

_{Human-Review-Need: 4 @ 2ee75bd}

[gemini-code-assist]

Code Review

This pull request addresses a critical issue where transactions dropped without explicit commit or abort were not being released, causing them to pin RocksDB snapshots and prevent the reclamation of obsolete versions. The changes introduce an onWrapperCollected finalizer mechanism to properly close orphaned transactions, ensure that in-flight asynchronous work is respected, and add diagnostic tools (such as rocksdb.num-snapshots and transaction age tracking) to monitor and debug these scenarios. I have reviewed the implementation and included a high-severity security recommendation regarding a potential Use-After-Free hazard in the asynchronous completion callback, which requires adjustment to ensure close() is only invoked when all concurrent operations have finished.

[kriszyp]

Could we strong reference (napi_ref) this (jsThis), which should entrain the transaction, to pin the transaction in memory, using the natural/native mechanism, rather than creating a separate mechanism for tracking?

_{🤖 Reviewed with Codex}

[kriszyp]

This case does not appear to fail on the pre-fix implementation. Both gets receive the same 250 ms delay, while close() waits up to five seconds: either both workers finish before the first completion runs, or the second finishes safely while the first completion waits. In both cases the old code still rejects both promises and removes the registry entry. Could we use a deterministic stagger or barrier so the first completion runs while the second worker remains blocked, assert that the transaction stays registered without blocking the event loop, then release the second worker and assert cleanup? That would specifically guard the new last-active-get invariant.

— KrAIs (Codex)