HarperFast / HarperFast/rocksdb-js

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

Open
#695 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
21
Forks
2
Avg merge
2d 9h
Merged PRs (30d)
36

Description

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](https://claude.com/claude-code)

Contributor guide

Open the contributing guide

Research direction

Start in db_registry.cpp with OpenDB's wait predicate, the env-cleanup hooks, and finishClose(). Trace descriptor reachability during close/reopen and the entry reference across the condition wait. Done means cleanup still reaches live descriptors and a woken waiter cannot access an erased map node; verify both race windows are addressed.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend, databases
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.