HarperFast / HarperFast/rocksdb-js

Audit DBHandle N-API references reachable from cross-environment shutdown

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

Description

Parent: #741

## Problem

PR #780 removes the transaction-owned `jsDatabaseRef`, making `TransactionHandle::close()` N-API-free. `DBHandle` still owns environment-bound references, including transaction-log entries in `logRefs`, and `DBHandle::close()` deletes those references unconditionally. That close path is reachable from shared-descriptor and registry shutdown, where the calling thread or teardown phase may not match the environment that created each reference.

PR #745 added a teardown guard around the `DBHandle::close()` loop, but the PR was closed and #780 does not carry an equivalent audit or fix.

## Required invariant

Every environment-owned N-API reference reachable from process-global or cross-environment native state must be released only through a lifecycle path that is valid for its owning environment. Cross-environment and late-teardown cleanup must not call N-API through a stale `napi_env`.

## Acceptance criteria

- Enumerate `DBHandle` and adjacent shutdown paths that create, read, or delete environment-owned N-API references.
- Establish ownership and allowed-thread/allowed-phase rules for each reference.
- Prefer environment-scoped release or structural removal of the native reference over scattered thread-identity checks.
- Verify `DBRegistry::Shutdown`, shared `DBDescriptor::finishClose`, user-called `db.close()`, GC finalization, and worker environment teardown.
- Add a worker test that exercises transaction-log references across shared-database environment churn and last-environment shutdown.

## Relationship

- Umbrella: #741
- Transaction-owned reference removal: #780
- Previous partial implementation/reference: #745

— KrAIs (Codex), on behalf of @kriszyp

Contributor guide

Open the contributing guide

Research direction

Start by auditing DBHandle and its logRefs references, then trace DBRegistry::Shutdown, shared DBDescriptor::finishClose, user-called db.close(), GC finalization, and worker environment teardown. Establish ownership and allowed lifecycle rules for each reference, then add the worker test covering shared-database environment churn and last-environment shutdown; done means all listed paths avoid stale-environment N-API cleanup.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, node.js
Domain
backend, database
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.