diff --git a/dist/src/store.js b/dist/src/store.js index 76b15060..268fde25 100644 --- a/dist/src/store.js +++ b/dist/src/store.js @@ -648,7 +648,12 @@ export class MemoryStore { } } } - async runWithWriteLock(fn) { + async runWithWriteLock(fn, options = {}) { + // 【MR2 fix】destroy() 之後拒絕新的寫入,給出明確錯誤而非 null-dereference。 + // destroy() 自己的 flush/teardown 透過 allowDestroyed 繞過此檢查。 + if (this.destroyed && options.allowDestroyed !== true) { + throw new Error("MemoryStore instance has been destroyed"); + } if (!this.redisLock) { return this.runWithFileLock(fn); } @@ -664,6 +669,25 @@ export class MemoryStore { this.config.onLockWarning?.(`memory-lancedb-pro: failed to close Redis lock client: ${String(err)}`); } } + /** + * 【MR1/MR3 fix】Explicitly close the native LanceDB table and connection so + * their resources are released immediately instead of waiting for GC, and + * surface failures instead of swallowing them in an empty catch block. + */ + closeNativeResources(table, db) { + try { + table?.close(); + } + catch (err) { + console.warn(`[memory-lancedb-pro] failed to close LanceDB table: ${err instanceof Error ? err.message : String(err)}`); + } + try { + db.close(); + } + catch (err) { + console.warn(`[memory-lancedb-pro] failed to close LanceDB connection: ${err instanceof Error ? err.message : String(err)}`); + } + } get dbPath() { return this.config.dbPath; } @@ -755,6 +779,11 @@ export class MemoryStore { /** Public so callers can warm the one-time table open / FTS index build * outside latency-sensitive windows (startup health probes do this). */ async ensureInitialized() { + // 【MR2 fix】destroy() 之後給出明確錯誤,而不是讓後續呼叫在 + // this.table === null 時拋出難以理解的 null-dereference。 + if (this.destroyed) { + throw new Error("MemoryStore instance has been destroyed"); + } if (this.table) { return; } @@ -1245,7 +1274,7 @@ export class MemoryStore { * @returns {hasError: boolean, lastError?: Error} — error info so callers * (flush/destroy) can rethrow without relying on shared instance state. */ - async doFlush() { + async doFlush(options = {}) { const prevLock = this.flushLock; let releaseLock; this.flushLock = new Promise((resolve) => { releaseLock = resolve; }); @@ -1271,7 +1300,7 @@ export class MemoryStore { try { await this.runWithWriteLock(async () => { await this.table.add(chunk.map((entry) => this.toRow(entry))); - }); + }, options); this.noteDataModification(); } catch (err) { @@ -1406,7 +1435,8 @@ export class MemoryStore { } // 【MR4 fix】設定 destroyed flag,阻止後續 bulkStore() 呼叫 this.destroyed = true; - const result = await this.doFlush(); + // destroy() 自己的收尾 flush 必須繞過 destroyed 寫入守衛。 + const result = await this.doFlush({ allowDestroyed: true }); // 【F1 fix】等待所有已排程的 timer callback 完成 // 透過 await flushLock 確保排隊中的 doFlush 都結束 // 防止:timer callback 已排程 → destroy() 清除 timer → destroy() 返回 @@ -1442,6 +1472,27 @@ export class MemoryStore { destroyError = err; } await this.closeLockResources(); + if (this.db) { + const db = this.db; + const table = this.table; + // 【F2 fix】在釋放 native 連線前先取得 write lock,確保任何 in-flight 寫入 + // (特別是 update 的 delete + re-add / rollback)先完成。update() 全程持有 + // write lock;若 destroy() 在 delete 與 rollback 之間把 this.table 設為 null + // 或關閉 connection,rollback 會失敗並造成資料遺失。 + try { + await this.runWithWriteLock(async () => { + await this.updateQueue; + }, { allowDestroyed: true }); + } + catch (err) { + console.warn(`[memory-lancedb-pro] failed to quiesce writes before destroy: ${err instanceof Error ? err.message : String(err)}`); + } + // 【MR1 fix】明確關閉 table/connection handle,讓 native 資源立即釋放, + // 而不是僅解除 JS 參考、等 GC 回收。【MR3 fix】close error 不再被靜默吞掉。 + this.closeNativeResources(table, db); + this.db = null; + this.table = null; + } if (destroyError) { throw destroyError; } @@ -2490,6 +2541,12 @@ export class MemoryStore { * supersede commit path calls it from inside its own atomic section. */ async performUpdateLocked(id, updates, scopeFilter) { + // 【F2 fix】此方法在 write lock 內執行。若 destroy() 於 update() 通過 + // ensureInitialized() 之後、取得鎖之前完成,這裡會給出明確錯誤, + // 而不是在已關閉的連線上做 delete/re-add。 + if (this.destroyed) { + throw new Error("MemoryStore instance has been destroyed"); + } // Support full UUID, short hex prefixes, and constrained exact legacy IDs imported // from older stores (for example "mem-md-..." or "data-pointer-..."). const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; diff --git a/package.json b/package.json index e8730ef6..48d36eb3 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,7 @@ "skills/**/*.md" ], "scripts": { - "test": "node test/embedder-error-hints.test.mjs && node --test test/embedder-max-input-chars.test.mjs && node test/cjk-recursion-regression.test.mjs && node test/extraction-prompt-structural-noise.test.mjs && node test/i18n-memory-triggers.test.mjs && node test/migrate-legacy-schema.test.mjs && node --test test/config-session-strategy-migration.test.mjs && node --test test/scope-access-undefined.test.mjs && node --test test/reflection-bypass-hook.test.mjs && node --test test/reflection-unattributed-session-read.test.mjs && node --test test/smart-extractor-scope-filter.test.mjs && node --test test/store-empty-scope-filter.test.mjs && node --test test/recall-text-cleanup.test.mjs && node --test test/native-recall.test.mjs && node test/update-consistency-lancedb.test.mjs && node --test test/strip-envelope-metadata.test.mjs && node test/cli-smoke.mjs && node test/functional-e2e.mjs && node --test test/per-agent-auto-recall.test.mjs && node test/retriever-rerank-regression.mjs && node test/smart-memory-lifecycle.mjs && node test/smart-extractor-branches.mjs && node --test test/smart-extractor-noise-gating.test.mjs && node test/memory-capability-runtime.test.mjs && node --test test/startup-health-diagnostics.test.mjs && node test/corpus-indexer.test.mjs && node --test test/regex-fallback-bulk-store.test.mjs && node test/plugin-manifest-regression.mjs && node --test test/dreaming-engine.test.mjs && node --test test/session-summary-before-reset.test.mjs && node --test test/sync-plugin-version.test.mjs && node test/smart-metadata-v2.mjs && node test/vector-search-cosine.test.mjs && node test/context-support-e2e.mjs && node test/temporal-facts.test.mjs && node test/memory-update-supersede.test.mjs && node test/memory-update-metadata-refresh.test.mjs && node test/memory-upgrader-diagnostics.test.mjs && node --test test/llm-api-key-client.test.mjs && node --test test/llm-oauth-client.test.mjs && node --test test/cli-oauth-login.test.mjs && node --test test/workflow-fork-guards.test.mjs && node --test test/clawteam-scope.test.mjs && node --test test/cross-process-lock.test.mjs && node --test test/preference-slots.test.mjs && node test/is-latest-auto-supersede.test.mjs && node --test test/temporal-awareness.test.mjs && node --test test/command-reflection-guard.test.mjs && node --test test/tier1-counters.test.mjs && node --test test/startup-check-timeout.test.mjs && node --test test/memory-subsession-prompt-hooks.test.mjs && node --test test/read-consistency-interval.test.mjs && node --test test/reflection-distiller-hook-skip.test.mjs && node --test test/register-scope-dedup.test.mjs && node --test test/raw-run-distiller-hooks.test.mjs && node --test test/autocapture-watermark-reset.test.mjs && node --test test/autocapture-internal-session-guard.test.mjs && node --test test/memory-categories-storage-map.test.mjs && node --test test/delete-invalidate-reflection-caches.test.mjs && node --test test/reflection-mapped-rows-admission.test.mjs && node --test test/smart-metadata-source-classification.test.mjs && node --test test/reflection-embed-transient-retry.test.mjs && node --test test/scope-owner-leak-hardening.test.mjs && node --test test/isOwnedByAgent.test.mjs && node --test test/typed-array-vector-fetch.test.mjs && node --test test/extraction-grounding-register.test.mjs && node test/grounding-rejudge.test.mjs && node --test test/reverse-map-legacy-category.test.mjs && node --test test/reflection-mapped-category-stamping.test.mjs && node --test test/memory-upgrader-category-normalization.test.mjs && node --test test/autocapture-fallback-gating.test.mjs && node --test test/prompt-architecture.test.mjs && node test/extraction-category-rubric.test.mjs && node --test test/admission-control-batch-utility.test.mjs && node --test test/smart-extractor-batch-admission.test.mjs && node --test test/admission-control-prompt-shape.test.mjs && node --test test/smart-extractor-merge-accounting.test.mjs && node --test test/admission-utility-veto.test.mjs && node --test test/cli-subcommand-attachment.test.mjs && node --test test/admission-lane-model-affinity.test.mjs && node --test test/admission-model-resolution.test.mjs && node --test test/admission-controller-standalone.test.mjs && node --test test/smart-extractor-admission-controller-injection.test.mjs && node --test test/admission-without-smart-extraction.test.mjs && node --test test/llm-thinklevel.test.mjs && node --test test/memory-id-prefix-resolution.test.mjs && node --test test/manual-store-supersede.test.mjs && node --test test/extraction-transcript-speaker-tags.test.mjs && node --test test/session-compressor.test.mjs && node --test test/reflection-derived-cache-invalidation.test.mjs && node --test test/reflection-tagged-input.test.mjs && node --test test/reflection-mapped-uniform-pipeline.test.mjs && node --test test/llm-host-transport.test.mjs && node --test test/llm-host-transport-composition.test.mjs && node --test test/admission-control-host-transport.test.mjs && node --test test/llm-transport-credential-hygiene.test.mjs && node --test test/llm-json-error-redaction.test.mjs && node --test test/memory-consolidate.test.mjs && node --test test/memory-consolidate-cost-gate.test.mjs && node --test test/memory-consolidate-two-phase-apply.test.mjs && node --test test/memory-consolidate-admission-independence.test.mjs && node --test test/memory-consolidate-polish.test.mjs && node --test test/consolidate-cli-settled-persistence.test.mjs && node --test test/consolidate-cli-apply-exit-status.test.mjs && node --test test/invalidated-rows-visibility.test.mjs && node --test test/store-excludeinactive-default.test.mjs && node --test test/manual-echo-guard.test.mjs && node --test test/reflection-hook-session-memory.test.mjs && node --test test/reflection-runner-embedded-agent.test.mjs && node --test test/reflection-before-reset-transcript.test.mjs && node --test test/mdmirror-workspace-map-entries.test.mjs && node --test test/openclaw-paths.test.mjs && node --test test/fts-search-text-column.test.mjs && node --test test/pair-window-retention.test.mjs && node --test test/auto-capture-cleanup.test.mjs && node --test test/cli-metadata-registration.test.mjs && node --test test/host-runtime-lifecycle-error.test.mjs && node --test test/autocapture-agent-end-owned-run.test.mjs && node --test test/memory-runtime-provenance-classification.test.mjs && node --test test/extraction-transient-retry.test.mjs && node --test test/autocapture-terminal-flush-retry.test.mjs && node --test test/auto-capture-unknown-watermark-window.test.mjs && node --test test/autocapture-unknown-watermark-injection.test.mjs", + "test": "node test/embedder-error-hints.test.mjs && node --test test/embedder-max-input-chars.test.mjs && node test/cjk-recursion-regression.test.mjs && node test/extraction-prompt-structural-noise.test.mjs && node test/i18n-memory-triggers.test.mjs && node test/migrate-legacy-schema.test.mjs && node --test test/config-session-strategy-migration.test.mjs && node --test test/scope-access-undefined.test.mjs && node --test test/reflection-bypass-hook.test.mjs && node --test test/reflection-unattributed-session-read.test.mjs && node --test test/smart-extractor-scope-filter.test.mjs && node --test test/store-empty-scope-filter.test.mjs && node --test test/recall-text-cleanup.test.mjs && node --test test/native-recall.test.mjs && node test/update-consistency-lancedb.test.mjs && node --test test/store-destroy-resources.test.mjs && node --test test/strip-envelope-metadata.test.mjs && node test/cli-smoke.mjs && node test/functional-e2e.mjs && node --test test/per-agent-auto-recall.test.mjs && node test/retriever-rerank-regression.mjs && node test/smart-memory-lifecycle.mjs && node test/smart-extractor-branches.mjs && node --test test/smart-extractor-noise-gating.test.mjs && node test/memory-capability-runtime.test.mjs && node --test test/startup-health-diagnostics.test.mjs && node test/corpus-indexer.test.mjs && node --test test/regex-fallback-bulk-store.test.mjs && node test/plugin-manifest-regression.mjs && node --test test/dreaming-engine.test.mjs && node --test test/session-summary-before-reset.test.mjs && node --test test/sync-plugin-version.test.mjs && node test/smart-metadata-v2.mjs && node test/vector-search-cosine.test.mjs && node test/context-support-e2e.mjs && node test/temporal-facts.test.mjs && node test/memory-update-supersede.test.mjs && node test/memory-update-metadata-refresh.test.mjs && node test/memory-upgrader-diagnostics.test.mjs && node --test test/llm-api-key-client.test.mjs && node --test test/llm-oauth-client.test.mjs && node --test test/cli-oauth-login.test.mjs && node --test test/workflow-fork-guards.test.mjs && node --test test/clawteam-scope.test.mjs && node --test test/cross-process-lock.test.mjs && node --test test/preference-slots.test.mjs && node test/is-latest-auto-supersede.test.mjs && node --test test/temporal-awareness.test.mjs && node --test test/command-reflection-guard.test.mjs && node --test test/tier1-counters.test.mjs && node --test test/startup-check-timeout.test.mjs && node --test test/memory-subsession-prompt-hooks.test.mjs && node --test test/read-consistency-interval.test.mjs && node --test test/reflection-distiller-hook-skip.test.mjs && node --test test/register-scope-dedup.test.mjs && node --test test/raw-run-distiller-hooks.test.mjs && node --test test/autocapture-watermark-reset.test.mjs && node --test test/autocapture-internal-session-guard.test.mjs && node --test test/memory-categories-storage-map.test.mjs && node --test test/delete-invalidate-reflection-caches.test.mjs && node --test test/reflection-mapped-rows-admission.test.mjs && node --test test/smart-metadata-source-classification.test.mjs && node --test test/reflection-embed-transient-retry.test.mjs && node --test test/scope-owner-leak-hardening.test.mjs && node --test test/isOwnedByAgent.test.mjs && node --test test/typed-array-vector-fetch.test.mjs && node --test test/extraction-grounding-register.test.mjs && node test/grounding-rejudge.test.mjs && node --test test/reverse-map-legacy-category.test.mjs && node --test test/reflection-mapped-category-stamping.test.mjs && node --test test/memory-upgrader-category-normalization.test.mjs && node --test test/autocapture-fallback-gating.test.mjs && node --test test/prompt-architecture.test.mjs && node test/extraction-category-rubric.test.mjs && node --test test/admission-control-batch-utility.test.mjs && node --test test/smart-extractor-batch-admission.test.mjs && node --test test/admission-control-prompt-shape.test.mjs && node --test test/smart-extractor-merge-accounting.test.mjs && node --test test/admission-utility-veto.test.mjs && node --test test/cli-subcommand-attachment.test.mjs && node --test test/admission-lane-model-affinity.test.mjs && node --test test/admission-model-resolution.test.mjs && node --test test/admission-controller-standalone.test.mjs && node --test test/smart-extractor-admission-controller-injection.test.mjs && node --test test/admission-without-smart-extraction.test.mjs && node --test test/llm-thinklevel.test.mjs && node --test test/memory-id-prefix-resolution.test.mjs && node --test test/manual-store-supersede.test.mjs && node --test test/extraction-transcript-speaker-tags.test.mjs && node --test test/session-compressor.test.mjs && node --test test/reflection-derived-cache-invalidation.test.mjs && node --test test/reflection-tagged-input.test.mjs && node --test test/reflection-mapped-uniform-pipeline.test.mjs && node --test test/llm-host-transport.test.mjs && node --test test/llm-host-transport-composition.test.mjs && node --test test/admission-control-host-transport.test.mjs && node --test test/llm-transport-credential-hygiene.test.mjs && node --test test/llm-json-error-redaction.test.mjs && node --test test/memory-consolidate.test.mjs && node --test test/memory-consolidate-cost-gate.test.mjs && node --test test/memory-consolidate-two-phase-apply.test.mjs && node --test test/memory-consolidate-admission-independence.test.mjs && node --test test/memory-consolidate-polish.test.mjs && node --test test/consolidate-cli-settled-persistence.test.mjs && node --test test/consolidate-cli-apply-exit-status.test.mjs && node --test test/invalidated-rows-visibility.test.mjs && node --test test/store-excludeinactive-default.test.mjs && node --test test/manual-echo-guard.test.mjs && node --test test/reflection-hook-session-memory.test.mjs && node --test test/reflection-runner-embedded-agent.test.mjs && node --test test/reflection-before-reset-transcript.test.mjs && node --test test/mdmirror-workspace-map-entries.test.mjs && node --test test/openclaw-paths.test.mjs && node --test test/fts-search-text-column.test.mjs && node --test test/pair-window-retention.test.mjs && node --test test/auto-capture-cleanup.test.mjs && node --test test/cli-metadata-registration.test.mjs && node --test test/host-runtime-lifecycle-error.test.mjs && node --test test/autocapture-agent-end-owned-run.test.mjs && node --test test/memory-runtime-provenance-classification.test.mjs && node --test test/extraction-transient-retry.test.mjs && node --test test/autocapture-terminal-flush-retry.test.mjs && node --test test/auto-capture-unknown-watermark-window.test.mjs && node --test test/autocapture-unknown-watermark-injection.test.mjs", "test:cli-smoke": "node scripts/run-ci-tests.mjs --group cli-smoke", "test:core-regression": "node scripts/run-ci-tests.mjs --group core-regression", "test:storage-and-schema": "node scripts/run-ci-tests.mjs --group storage-and-schema", diff --git a/scripts/ci-test-manifest.mjs b/scripts/ci-test-manifest.mjs index cba5ca7f..2e3f704f 100644 --- a/scripts/ci-test-manifest.mjs +++ b/scripts/ci-test-manifest.mjs @@ -33,6 +33,7 @@ export const CI_TEST_MANIFEST = [ { group: "core-regression", runner: "node", file: "test/openclaw-paths.test.mjs", args: ["--test"] }, { group: "core-regression", runner: "node", file: "test/category-filter-normalization.test.mjs", args: ["--test"] }, { group: "storage-and-schema", runner: "node", file: "test/update-consistency-lancedb.test.mjs" }, + { group: "storage-and-schema", runner: "node", file: "test/store-destroy-resources.test.mjs", args: ["--test"] }, { group: "core-regression", runner: "node", file: "test/strip-envelope-metadata.test.mjs", args: ["--test"] }, { group: "core-regression", runner: "node", file: "test/auto-recall-timeout.test.mjs", args: ["--test"] }, { group: "cli-smoke", runner: "node", file: "test/import-markdown/import-markdown.test.mjs", args: ["--test"] }, diff --git a/src/store.ts b/src/store.ts index 0335788e..ab3a5d45 100644 --- a/src/store.ts +++ b/src/store.ts @@ -828,7 +828,12 @@ export class MemoryStore { } } - private async runWithWriteLock(fn: () => Promise): Promise { + private async runWithWriteLock(fn: () => Promise, options: { allowDestroyed?: boolean } = {}): Promise { + // 【MR2 fix】destroy() 之後拒絕新的寫入,給出明確錯誤而非 null-dereference。 + // destroy() 自己的 flush/teardown 透過 allowDestroyed 繞過此檢查。 + if (this.destroyed && options.allowDestroyed !== true) { + throw new Error("MemoryStore instance has been destroyed"); + } if (!this.redisLock) { return this.runWithFileLock(fn); } @@ -847,6 +852,31 @@ export class MemoryStore { } } + /** + * 【MR1/MR3 fix】Explicitly close the native LanceDB table and connection so + * their resources are released immediately instead of waiting for GC, and + * surface failures instead of swallowing them in an empty catch block. + */ + private closeNativeResources( + table: LanceDB.Table | null, + db: LanceDB.Connection, + ): void { + try { + table?.close(); + } catch (err) { + console.warn( + `[memory-lancedb-pro] failed to close LanceDB table: ${err instanceof Error ? err.message : String(err)}`, + ); + } + try { + db.close(); + } catch (err) { + console.warn( + `[memory-lancedb-pro] failed to close LanceDB connection: ${err instanceof Error ? err.message : String(err)}`, + ); + } + } + get dbPath(): string { return this.config.dbPath; } @@ -950,6 +980,11 @@ export class MemoryStore { /** Public so callers can warm the one-time table open / FTS index build * outside latency-sensitive windows (startup health probes do this). */ async ensureInitialized(): Promise { + // 【MR2 fix】destroy() 之後給出明確錯誤,而不是讓後續呼叫在 + // this.table === null 時拋出難以理解的 null-dereference。 + if (this.destroyed) { + throw new Error("MemoryStore instance has been destroyed"); + } if (this.table) { return; } @@ -1509,7 +1544,7 @@ export class MemoryStore { * @returns {hasError: boolean, lastError?: Error} — error info so callers * (flush/destroy) can rethrow without relying on shared instance state. */ - private async doFlush(): Promise<{ hasError: boolean; lastError?: Error }> { + private async doFlush(options: { allowDestroyed?: boolean } = {}): Promise<{ hasError: boolean; lastError?: Error }> { const prevLock = this.flushLock; let releaseLock: () => void; this.flushLock = new Promise((resolve) => { releaseLock = resolve; }); @@ -1538,7 +1573,7 @@ export class MemoryStore { try { await this.runWithWriteLock(async () => { await this.table!.add(chunk.map((entry) => this.toRow(entry))); - }); + }, options); this.noteDataModification(); } catch (err) { lastError = err as Error; @@ -1680,7 +1715,8 @@ export class MemoryStore { } // 【MR4 fix】設定 destroyed flag,阻止後續 bulkStore() 呼叫 this.destroyed = true; - const result = await this.doFlush(); + // destroy() 自己的收尾 flush 必須繞過 destroyed 寫入守衛。 + const result = await this.doFlush({ allowDestroyed: true }); // 【F1 fix】等待所有已排程的 timer callback 完成 // 透過 await flushLock 確保排隊中的 doFlush 都結束 @@ -1722,6 +1758,28 @@ export class MemoryStore { } await this.closeLockResources(); + if (this.db) { + const db = this.db; + const table = this.table; + // 【F2 fix】在釋放 native 連線前先取得 write lock,確保任何 in-flight 寫入 + // (特別是 update 的 delete + re-add / rollback)先完成。update() 全程持有 + // write lock;若 destroy() 在 delete 與 rollback 之間把 this.table 設為 null + // 或關閉 connection,rollback 會失敗並造成資料遺失。 + try { + await this.runWithWriteLock(async () => { + await this.updateQueue; + }, { allowDestroyed: true }); + } catch (err) { + console.warn( + `[memory-lancedb-pro] failed to quiesce writes before destroy: ${err instanceof Error ? err.message : String(err)}`, + ); + } + // 【MR1 fix】明確關閉 table/connection handle,讓 native 資源立即釋放, + // 而不是僅解除 JS 參考、等 GC 回收。【MR3 fix】close error 不再被靜默吞掉。 + this.closeNativeResources(table, db); + this.db = null; + this.table = null; + } if (destroyError) { throw destroyError; } @@ -2985,6 +3043,12 @@ export class MemoryStore { updates: MemoryUpdatePatch, scopeFilter?: string[], ): Promise { + // 【F2 fix】此方法在 write lock 內執行。若 destroy() 於 update() 通過 + // ensureInitialized() 之後、取得鎖之前完成,這裡會給出明確錯誤, + // 而不是在已關閉的連線上做 delete/re-add。 + if (this.destroyed) { + throw new Error("MemoryStore instance has been destroyed"); + } // Support full UUID, short hex prefixes, and constrained exact legacy IDs imported // from older stores (for example "mem-md-..." or "data-pointer-..."). const uuidRegex = diff --git a/test/store-destroy-resources.test.mjs b/test/store-destroy-resources.test.mjs new file mode 100644 index 00000000..40450ddd --- /dev/null +++ b/test/store-destroy-resources.test.mjs @@ -0,0 +1,198 @@ +// test/store-destroy-resources.test.mjs +/** + * Verifies destroy() releases native LanceDB resources safely. + * + * Must-fix F2: destroy() must wait for an in-flight update (delete + re-add, + * with rollback) instead of nulling/closing the table between the delete and + * its re-add, which would lose the row. + * MR1: the native table handle is closed explicitly instead of waiting for GC. + * MR2: using the store after destroy() throws a clear "destroyed" error. + * MR3: a failing native close is reported instead of swallowed. + */ + +import { describe, it, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import jitiFactory from "jiti"; + +const jiti = jitiFactory(import.meta.url, { interopDefault: true }); +const { MemoryStore } = jiti("../src/store.ts"); + +function deferred() { + let resolve; + let reject; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +describe("MemoryStore.destroy resource cleanup (real LanceDB backend)", () => { + let workDir; + + afterEach(() => { + if (workDir) rmSync(workDir, { recursive: true, force: true }); + }); + + async function createStoreWithEntry(overrides = {}) { + workDir = mkdtempSync(path.join(tmpdir(), "memory-lancedb-destroy-")); + const store = new MemoryStore({ + dbPath: path.join(workDir, "db"), + vectorDim: 4, + }); + + const entry = await store.store({ + text: "original memory", + vector: [0, 0, 0, 0], + category: "fact", + scope: "global", + importance: 0.7, + metadata: "{}", + ...overrides, + }); + + return { store, entry }; + } + + function wrapTableMethod(store, methodName, wrapper) { + const table = store.table; + assert.ok(table, `expected initialized table for ${methodName}`); + const original = table[methodName].bind(table); + table[methodName] = wrapper(original); + return () => { + table[methodName] = original; + }; + } + + it("waits for an in-flight update before closing the connection (F2)", async () => { + const { store, entry } = await createStoreWithEntry(); + + const addStarted = deferred(); + const addGate = deferred(); + let addCount = 0; + let reAddCompleted = false; + const restore = wrapTableMethod(store, "add", (original) => async (...args) => { + addCount += 1; + if (addCount === 1) { + addStarted.resolve(); + await addGate.promise; + } + const result = await original(...args); + if (addCount === 1) reAddCompleted = true; + return result; + }); + + // Spy on the native connection close so we can assert destroy() did not + // tear it down while the update was still between delete and re-add. + let closedWhileReAddInFlight = null; + const db = store.db; + assert.ok(db, "expected initialized connection"); + const originalDbClose = db.close.bind(db); + db.close = () => { + closedWhileReAddInFlight = !reAddCompleted; + return originalDbClose(); + }; + + const updatePromise = store.update(entry.id, { + text: "updated memory", + vector: [1, 1, 1, 1], + }); + + await addStarted.promise; // update is now between its delete and re-add + + const destroyPromise = store.destroy(); + addGate.resolve(); // let the in-flight re-add finish + + const updated = await updatePromise; + assert.equal(updated?.text, "updated memory"); + + await destroyPromise; + + assert.equal( + closedWhileReAddInFlight, + false, + "destroy() closed the connection while an update was still in flight", + ); + + restore(); + }); + + it("closes the native table handle and clears references (MR1)", async () => { + const { store } = await createStoreWithEntry(); + + const table = store.table; + assert.ok(table, "expected initialized table"); + + let closed = 0; + const originalClose = table.close.bind(table); + table.close = () => { + closed += 1; + return originalClose(); + }; + + await store.destroy(); + + assert.equal(closed, 1, "expected the native table handle to be closed once"); + assert.equal(store.table, null); + assert.equal(store.db, null); + }); + + it("throws a clear error when used after destroy (MR2)", async () => { + const { store, entry } = await createStoreWithEntry(); + + await store.destroy(); + + await assert.rejects( + () => store.getById(entry.id), + /MemoryStore instance has been destroyed/, + ); + await assert.rejects( + () => store.update(entry.id, { text: "nope" }), + /MemoryStore instance has been destroyed/, + ); + await assert.rejects( + () => + store.store({ + text: "nope", + vector: [1, 1, 1, 1], + category: "fact", + scope: "global", + importance: 0.5, + metadata: "{}", + }), + /MemoryStore instance has been destroyed/, + ); + }); + + it("reports native close failures instead of swallowing them (MR3)", async () => { + const { store } = await createStoreWithEntry(); + + const table = store.table; + assert.ok(table, "expected initialized table"); + const originalClose = table.close.bind(table); + table.close = () => { + throw new Error("injected table close failure"); + }; + + const warnings = []; + const originalWarn = console.warn; + console.warn = (...args) => { + warnings.push(args.join(" ")); + }; + try { + await store.destroy(); + } finally { + console.warn = originalWarn; + table.close = originalClose; + } + + assert.ok( + warnings.some((message) => /failed to close LanceDB table/.test(message)), + `expected a close failure warning, got: ${JSON.stringify(warnings)}`, + ); + assert.equal(store.table, null); + }); +});