Close one database alias without tearing down the store its other aliases share

DESIGN.md

@@ -42,6 +42,7 @@ Index of the design notes for the harper core: one line per note, grouped by the
 - [Audit-entry removal loops must track every `removeAuditEntry()`/`removeEntry()` promise](resources/DESIGN.md#audit-entry-removal-loops-must-track-every-removeauditentryremoveentry-promise) — Every removal promise in a batch loop needs a rejection handler attached immediately.
 - [Audit retention cleanup is a self-rearming, engine-independent lifecycle](resources/DESIGN.md#audit-retention-cleanup-is-a-self-rearming-engine-independent-lifecycle) — One `scheduleAuditCleanup` call re-arms until the root store closes; engines differ in the work per pass, not in the lifecycle.
 - [`createBlob(readable)` and `table.put()` don't synchronously drain the source](resources/DESIGN.md#createblobreadable-and-tableput-dont-synchronously-drain-the-source) — `table.put()` returns before a `Readable`-backed blob has drained; finalize a hash on the stream's end or await `storageInfo.saving`.
+- [A shared root store closes with its last alias, never its first](resources/DESIGN.md#a-shared-root-store-closes-with-its-last-alias-never-its-first-databasests-closedatabaseclosedatabasewithaliases) — `closeDatabase` releases one name; the root store (and its LMDB handles) close only when no other loaded name still references it, or a second alias's close use-after-frees the environment.
 - [Table drops, the `dropping` tombstone, and ghost tables](resources/DESIGN.md#table-drops-the-dropping-tombstone-and-ghost-tables) — `dropTable()` persists a `dropping` tombstone before destructive work; boot and a same-name create complete an interrupted drop.
 - [The exclusive `update-attributes` lock is a bounded synchronous wait, and drop-then-recreate needs the column-family eviction fix (`Table.ts`)](resources/DESIGN.md#the-exclusive-update-attributes-lock-is-a-bounded-synchronous-wait-and-drop-then-recreate-needs-the-column-family-eviction-fix-tablets) — The lock is a bounded `Atomics.wait` with structural release, acquired before any live-`Table` mutation; drop-then-recreate needs the `@harperfast/rocksdb-js` column-family eviction fix.
 - [RocksDB transaction log purges are database-wide only (`ResourceBridge.deleteTransactionLogsBefore`)](resources/DESIGN.md#rocksdb-transaction-log-purges-are-database-wide-only-resourcebridgedeletetransactionlogsbefore) — RocksDB transaction logs are per database, so a table-scoped `delete_transaction_logs_before` is rejected with 400.

resources/DESIGN.md

@@ -794,6 +794,29 @@ Consequence for callers that wrap the source in a hashing `Transform`: calling `
 
 Future agents touching `components/deploymentRecorder.ts` for Slice B's streaming variant should pick one of the latter two patterns.
 
+## A shared root store closes with its last alias, never its first (`databases.ts` `closeDatabase`/`closeDatabaseWithAliases`)
+
+Two database names that resolve to one path (a configured alias plus the generic scan, or two
+configured aliases) share one root store object, cached per path in `lmdbDatabaseEnvs`/
+`rocksdbDatabaseEnvs`. `closeDatabase(name)` (`:2222`) is therefore a **logical** release of one
+name only: it retires the name's table runtime (`Table.cleanup()`), closes the RocksDB
+column-family handles that name opened (each open is its own refcount), and tears the root store
+itself down — audit-cleanup stop, storage-reclamation unregistration, env-cache entry — only when
+`isRootStoreReferencedElsewhere` (`:2336`) finds no other loaded name still pointing at it. LMDB
+table handles are never closed individually: `mdb_dbi_close` invalidates the environment-wide slot
+for every wrapper of it and is optional by LMDB's contract, so closing one alias while another still
+holds a handle into the same environment is a use-after-free once the environment itself closes
+(`mdb_env_close` frees it), allocator-state dependent (harper#2721).
+
+`closeDatabaseWithAliases(name)` (`:2292`) is the **physical** close: every loaded name sharing a
+root store with `name`, waited on the derived-index runtime of every affected table to stop first
+so a flush in flight cannot land after the stores close. `restore_backup`'s ITC handler
+(`server/itc/serverHandlers.js`) calls this, not `closeDatabase`, because `verifyDatabaseClosed`
+polls the process-wide registry by path — an alias `closeDatabase` left open still blocks the purge.
+Root-store identity is compared by object, never by path: alias discovery only sees names already
+loaded on the current thread, which is sufficient because names load per thread all at once from
+one configuration scan.
+
 ## Table drops, the `dropping` tombstone, and ghost tables
 
 A table is a set of RocksDB column families (`T/` plus `T/<attr>`) and a set of catalog rows

resources/databases.ts

@@ -2211,49 +2211,60 @@ export async function dropDatabase(databaseName) {
 }
 
 /**
- * Close a RocksDB database's store handles on the current thread and unregister it, without
- * touching its files. Used by the restore_backup flow: every thread must release its handles so
- * `backups.restore()` can purge and rewrite the (fully closed) database directory. A subsequent
- * `resetDatabases()`/`getDatabases()` rescan reloads it (or skips it while a restore is in
- * progress, per the restore marker checks in the scan).
+ * Unregister one database name on the current thread and release what that name holds, without
+ * touching any files: its tables' process-wide registrations (timers, reclamation and delete
+ * callbacks, derived-index runtime) and, for RocksDB, the column-family handles it opened. The root
+ * store's native handles close with the last name that references it — another name resolving to
+ * the same path keeps the store open — so a caller that needs the store physically closed uses
+ * `closeDatabaseWithAliases`. A subsequent `resetDatabases()`/`getDatabases()` rescan reloads the
+ * name (or skips it while a restore is in progress, per the restore marker checks in the scan).
  */
 export function closeDatabase(databaseName: string): boolean {
 	const dbTables = databases[databaseName];
 	if (!dbTables) return false;
-	const rootStores = new Set<any>();
 	const closeStore = (store: any, description: string) => {
+		const warn = (error: unknown) =>
+			logger.warn(`Error closing ${description} while closing database ${databaseName}:`, error);
 		try {
-			store?.close?.();
+			const closing = store?.close?.();
+			if (typeof closing?.catch === 'function') closing.catch(warn);
 		} catch (error) {
-			logger.warn(`Error closing ${description} while closing database ${databaseName}:`, error);
+			warn(error);
 		}
 	};
-	for (const tableName in dbTables) {
-		const table: any = dbTables[tableName];
-		if (!table?.primaryStore) continue;
-		if (table.primaryStore.rootStore) rootStores.add(table.primaryStore.rootStore);
+	const rootStores = collectRootStores(databaseName);
+	const sharedRootStores = new Set<any>();
+	for (const rootStore of rootStores) {
+		if (isRootStoreReferencedElsewhere(rootStore, databaseName)) sharedRootStores.add(rootStore);
 	}
-	// a database with no tables (an empty schema, or one whose tables were all dropped) still holds
-	// an open root store, tracked only on the defined-database entry rather than any table — include
-	// it so its handles are released too (the Set dedupes it against the per-table root stores above)
-	const definedRoot = (definedDatabases?.get(databaseName) as any)?.rootStore;
-	if (definedRoot) rootStores.add(definedRoot);
 	// before any table store closes, so no further pass is admitted. This is synchronous, so it cannot
 	// await the drain barrier stopAuditCleanup() returns; what covers it is the in-pass status checks,
 	// plus the fact that its production callers reach it only for RocksDB databases, whose pass is one
 	// synchronous purgeLogs() call with nothing suspended mid-removal.
-	for (const rootStore of rootStores) rootStore.auditStore?.stopAuditCleanup?.();
+	for (const rootStore of rootStores) {
+		if (!sharedRootStores.has(rootStore)) (rootStore as any).auditStore?.stopAuditCleanup?.();
+	}
 	for (const tableName in dbTables) {
 		const table: any = dbTables[tableName];
 		if (!table?.primaryStore) continue;
+		try {
+			table.cleanup?.();
+		} catch (error) {
+			logger.warn(`Error retiring table ${tableName} while closing database ${databaseName}:`, error);
+		}
+		// a RocksDB column family handle is refcounted per open, so each name closes the ones it opened;
+		// an LMDB table handle is a slot of the environment (mdb_dbi_close invalidates it for every
+		// wrapper of it), so LMDB handles are released only by the environment close below
+		if (!(table.primaryStore.rootStore instanceof RocksDatabase)) continue;
 		for (const indexName in table.indices || {}) {
 			closeStore(table.indices[indexName], `index ${tableName}.${indexName}`);
 		}
 		closeStore(table.primaryStore, `table ${tableName}`);
 	}
 	for (const rootStore of rootStores) {
+		if (sharedRootStores.has(rootStore)) continue;
 		removeStorageReclamation(rootStore.path);
-		closeStore(rootStore.dbisDb, 'attributes store');
+		if (rootStore instanceof RocksDatabase) closeStore(rootStore.dbisDb, 'attributes store');
 		closeStore(rootStore, 'root store');
 		lmdbDatabaseEnvs.delete(rootStore.path);
 		rocksdbDatabaseEnvs.delete(rootStore.path);
@@ -2270,6 +2281,65 @@ export function closeDatabase(databaseName: string): boolean {
 	return true;
 }
 
+/**
+ * Close every loaded name that shares a root store with `databaseName`, `databaseName` included, so
+ * the store's native handles are actually released — what a restore needs before it replaces the
+ * files, where `closeDatabase` alone would leave the store open under its other names. Waits for
+ * every table's derived-index runtime to stop before the stores close, so a flush still in flight
+ * does not land after the files are gone; a stop that cannot prove its queued work quiescent is
+ * logged and the close proceeds.
+ */
+export async function closeDatabaseWithAliases(databaseName: string): Promise<boolean> {
+	if (!databases[databaseName]) return false;
+	const rootStores = collectRootStores(databaseName);
+	const names = [databaseName];
+	for (const otherName of Object.keys(databases)) {
+		if (otherName === databaseName) continue;
+		for (const rootStore of collectRootStores(otherName)) {
+			if (rootStores.has(rootStore)) {
+				names.push(otherName);
+				break;
+			}
+		}
+	}
+	const stops: Promise<unknown>[] = [];
+	for (const name of names) {
+		for (const tableName in databases[name]) {
+			const runtime = (databases[name][tableName] as any)?.derivedIndexRuntime;
+			if (runtime?.close) {
+				stops.push(
+					Promise.resolve(runtime.close()).catch((error) =>
+						logger.warn(`Error stopping the derived-index runtime of ${name}.${tableName}:`, error)
+					)
+				);
+			}
+		}
+	}
+	await Promise.all(stops);
+	for (const name of names) closeDatabase(name);
+	return true;
+}
+
+function collectRootStores(databaseName: string): Set<RootDatabaseKind> {
+	const rootStores = new Set<RootDatabaseKind>();
+	for (const tableName in databases[databaseName]) {
+		const rootStore = (databases[databaseName][tableName] as any)?.primaryStore?.rootStore;
+		if (rootStore) rootStores.add(rootStore);
+	}
+	// a database with no tables (an empty schema, or one whose tables were all dropped) still holds
+	// an open root store, tracked only on the defined-database entry rather than any table
+	const definedRoot = (definedDatabases?.get(databaseName) as any)?.rootStore;
+	if (definedRoot) rootStores.add(definedRoot);
+	return rootStores;
+}
+
+function isRootStoreReferencedElsewhere(rootStore: RootDatabaseKind, databaseName: string): boolean {
+	for (const otherName in databases) {
+		if (otherName !== databaseName && collectRootStores(otherName).has(rootStore)) return true;
+	}
+	return false;
+}
+
 /**
  * Close every RocksDB (user) database this thread has open, releasing its native handles.
  *

server/itc/serverHandlers.js

@@ -15,7 +15,7 @@ const { isMainThread, threadId, workerData } = require('node:worker_threads');
 const {
 	databases,
 	resetDatabases,
-	closeDatabase,
+	closeDatabaseWithAliases,
 	reloadBranchAt,
 	markDropInProgress,
 } = require('../../resources/databases.ts');
@@ -52,11 +52,12 @@ async function schemaHandler(event) {
 	}
 
 	hdbLogger.trace(`ITC schemaHandler received schema event:`, event);
-	// restore_backup: this thread must release its store handles so the restore can purge and
-	// rewrite the database directory. The rescan below (resetDatabases) skips reloading it while
-	// the restoring marker is present, and reloads it on the completion signal (marker gone).
+	// restore_backup: this thread must release its store handles — under every name that resolves
+	// to the database's path — so the restore can purge and rewrite the directory. The rescan below
+	// (resetDatabases) skips reloading it while the restoring marker is present, and reloads it on
+	// the completion signal (marker gone).
 	if (event.message?.operation === hdbTerms.OPERATIONS_ENUM.RESTORE_BACKUP && event.message.schema) {
-		closeDatabase(event.message.schema);
+		await closeDatabaseWithAliases(event.message.schema);
 	}
 	await cleanLmdbMap(event.message);
 	await syncSchemaMetadata(event.message);

unitTests/resources/databaseAliasIdentity-close.js

@@ -0,0 +1,54 @@
+// Child-process half of databaseAliasIdentity.test.js: closes two names of one store in the given
+// order under a private root, so the parent can run it with the allocator perturbed and read a
+// native fault as a non-zero exit. The mocha glob loads it too, hence the entry guard.
+'use strict';
+const { mkdirSync } = require('node:fs');
+const { join } = require('node:path');
+
+if (require.main === module) {
+	const [rootPath, storageRoot, firstToClose, secondToClose] = process.argv.slice(2);
+	const env = require('#src/utility/environment/environmentManager');
+	const terms = require('#src/utility/hdbTerms');
+	// A private root keeps this process off the parent's system database (RocksDB's lock is per
+	// process); only the store under test is shared, at the path the parent chose.
+	env.setProperty(terms.HDB_SETTINGS_NAMES.HDB_ROOT_KEY, rootPath);
+	env.setProperty(terms.CONFIG_PARAMS.STORAGE_PATH, join(rootPath, 'database'));
+	env.setProperty(terms.CONFIG_PARAMS.DATABASES, { physicalalias: { path: storageRoot } });
+	const { table, closeDatabase, resetDatabases } = require('#src/resources/databases');
+	const { setMainIsWorker } = require('#js/server/threads/manageThreads');
+	setMainIsWorker(true);
+	mkdirSync(join(rootPath, 'database'), { recursive: true });
+	mkdirSync(storageRoot, { recursive: true });
+
+	(async () => {
+		const Physical = table({
+			database: 'physicalalias',
+			table: 'CloseOrder',
+			attributes: [
+				{ name: 'id', isPrimaryKey: true },
+				{ name: 'group', indexed: true },
+			],
+		});
+		await Physical.put({ id: 'a', group: 'g' });
+		closeDatabase('physicalalias');
+		env.setProperty(terms.CONFIG_PARAMS.DATABASES, {
+			configuredalias: { path: storageRoot },
+			physicalalias: { path: storageRoot },
+		});
+		const databases = resetDatabases();
+		// a read through each name is what binds its table handle to the environment slot the
+		// other name's close then invalidates
+		for (const name of [firstToClose, secondToClose]) {
+			if ((await databases[name]?.CloseOrder.get('a'))?.group !== 'g')
+				throw new Error(`${name} must read the shared store`);
+		}
+		closeDatabase(firstToClose);
+		closeDatabase(secondToClose);
+	})().then(
+		() => process.exit(0),
+		(error) => {
+			console.error(error);
+			process.exit(1);
+		}
+	);
+}