HarperFast / HarperFast/rocksdb-js
DestroyDB wakes a parked OpenDB before the files are removed: the opener recreates the database and orphans a descriptor that holds the LOCK for the life of the process
- Dominant language
- C++
- Stars
- 21
- Forks
- 2
- Avg merge
- 2d 9h
- Merged PRs (30d)
- 36
Description
## Summary
`DBRegistry::DestroyDB` erases the path's registry entries and notifies waiters **before** it calls `rocksdb::DestroyDB` and `std::filesystem::remove_all`. An `OpenDB` on another thread that was parked on that path's condition (it saw the descriptor `isClosing()`) wakes at the notify and reopens the path — with `create_if_missing` (`db_descriptor.cpp:923`) — while or after the files are removed, so the destroyed database comes back as a fresh, empty one. Because that woken `OpenDB` still holds `auto& entry = entryIterator->second` into the node `DestroyDB` just erased (the dangling reference #695 records as "hazard 2"), it stores the new descriptor into freed memory and the descriptor is registered nowhere: no `CloseDB` can ever reach it, so the RocksDB `LOCK` it holds is held for the life of the process, and every later `OpenDB` of that path in the process fails with
```
IO error: lock hold by current process, acquire time acquiring thread : /LOCK: No locks available
```
Observed in Harper's CI, where it took every job worker thread of the process down (details below). Present in 2.8.0 (`db_registry.cpp` 190–200 erase+notify, 232–238 destroy, 289–300 the waiter) and in 2.7.1 (same shape at 190/197, 229/235, 286–293), which is what the harper `v5.2` line pins.
## Sequence
Thread A holds the database open; thread B holds it open too (shared descriptor).
1. A: `DestroyDB(path)` — `beginClose()` claims the descriptor, `finishClose()` closes every attached handle (B's included).
2. B: `OpenDB(path, {name: cf})` — finds the entry, sees `isClosing()`, resets `entry.descriptor` and waits on `entry.condition` holding `auto& entry` into the map node.
3. A: erases every entry for `path` (frees B's node), `notify_all()`.
4. A: `descriptor.reset()` → RocksDB closes, `LOCK` released.
5. Either order of the next two:
- A: `rocksdb::DestroyDB(path)` + `remove_all(path)`.
- B: wakes, reads `entry.descriptor` from freed storage (null), calls `DBDescriptor::open(path)` → `create_if_missing` → a new RocksDB at `path`, `LOCK` acquired by B's thread → stored into the freed node. Not in the map.
6. B's next `OpenDB(path)` (or anyone's): no entry → `DBDescriptor::open` → `PosixEnv::LockFile` finds the path in its in-process lock table → the error above, forever. If B won step 5, A's `rocksdb::DestroyDB` fails with the same error instead; if A won, the directory is recreated by B after the destroy.
`PurgeIfUnreferenced` (112–118) erases the node under the same waiter, so the plain last-close path has the dangling-reference half of this too.
## Reproduction (from Harper)
HarperFast/harper Integration Tests run 33592149855, job "Integration Tests 2/6 (uWS HTTP)", `integrationTests/apiTests/terminology.test.mjs`, `hdb.log` (process 2375):
```
04:50:31.909Z [http/1] Completing interrupted drop of table tuckerdoodle.todo ← thread B opening column stores of tuckerdoodle
04:50:32.022Z [main/0] { operation: 'drop_database', database: 'tuckerdoodle' } ← thread A: close() + destroy()
04:50:32.184Z [http/1] Failed to complete interrupted drop ... Error: IO error: lock hold by current process, acquire time 1788324632 acquiring thread 2645: .../database/tuckerdoodle/LOCK: No locks available
at Store.open (node_modules/@harperfast/rocksdb-js/src/store.ts:1142:11)
04:50:32.249Z [main/0] Error: IO error: lock hold by current process, acquire time 1788324632 acquiring thread 2645 ... opening database .../tuckerdoodle
... every later open of that path by every thread, 04:50 → 04:55, same acquire time and thread ...
04:50:32.853Z [main/0] Worker index undefined error: Error: IO error: lock hold by current process ... (job worker died at boot)
```
`acquire time 1788324632` is 04:50:32Z — the moment of the destroy — and the holder thread never changes: the orphaned descriptor from step 5. `drop_database` itself returned success (A won step 5), and the directory was present again on disk for every subsequent scan. Nine job-based tests then timed out because each job worker's boot opened the path and died.
A direct test: open a database on two worker threads; on thread A call `destroy()` while thread B calls `RocksDatabase.open(path, {name: 'cf'})` in a tight loop; afterwards assert that `open(path)` succeeds on a third thread and that `path` does not exist (or is exactly what the third open created). Today the third open fails with the lock error, or `path` exists before it.
## Proposed fix
- `DestroyDB`: keep the entry in the map, descriptor non-null and `isClosing()`, through `rocksdb::DestroyDB` and `remove_all`; erase and `notify_all()` only after the files are gone. Same discipline the comment above the claim block already describes ("keeps a concurrent OpenDB waiting on the entry's condition instead of re-opening the path while its files are being destroyed") — the erase just happens too early for that to be true.
- `OpenDB`: re-find the entry after the wait (never hold a reference into the map across it), and treat "entry gone after wake" as "start a fresh entry". Fixes #695's dangling reference in the same place.
- `PurgeIfUnreferenced`: the same re-find on the waiter side covers its guarded erase.
Whether a woken opener should *create* a database at a path that was just destroyed is a separate question for the caller (Harper will stop opening a database it is dropping — HarperFast/harper PR to follow); the registry's job is to never lose a descriptor.
Refs #695 (dangling entry reference), #808 (another `remove_all`-orphans-a-handle shape).
Contributor guide
Research direction
Start with DBRegistry::DestroyDB and the waiting path in db_registry.cpp around the referenced lines, then inspect DBDescriptor::open in db_descriptor.cpp:923. Reproduce the two-thread destroy/open sequence described in the issue and verify that destruction leaves no orphaned descriptor or lock, while a later open behaves as expected.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, node.js
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100