From 9dacb52aa724c0955fe300549aa9547b5053d532 Mon Sep 17 00:00:00 2001 From: Robin McCollum Date: Thu, 30 Oct 2025 14:52:33 -0700 Subject: [PATCH 1/3] Our existing code created prepared statements without finalizing them. This changes those to properly dispose of them. A future improvement is to only prepare the EXPORT statement once, and attach it to the Session or store. --- packages/memory/space.ts | 147 ++++++++++++++++++++++----------------- 1 file changed, 85 insertions(+), 62 deletions(-) diff --git a/packages/memory/space.ts b/packages/memory/space.ts index 9223b0620..8bca0238b 100644 --- a/packages/memory/space.ts +++ b/packages/memory/space.ts @@ -414,24 +414,29 @@ const recall = ( { store }: Session, { the, of }: { the: MIME; of: URI }, ): Revision | null => { - const row = store.prepare(EXPORT).get({ the, of }) as StateRow | undefined; - if (row) { - const revision: Revision = { - the, - of, - cause: row.cause - ? (fromString(row.cause) as Reference) - : refer(unclaimed({ the, of })), - since: row.since, - }; + const stmt = store.prepare(EXPORT); + try { + const row = stmt.get({ the, of }) as StateRow | undefined; + if (row) { + const revision: Revision = { + the, + of, + cause: row.cause + ? (fromString(row.cause) as Reference) + : refer(unclaimed({ the, of })), + since: row.since, + }; - if (row.is) { - revision.is = JSON.parse(row.is); - } + if (row.is) { + revision.is = JSON.parse(row.is); + } - return revision; - } else { - return null; + return revision; + } else { + return null; + } + } finally { + stmt.finalize(); } }; @@ -457,20 +462,25 @@ const _causeChain = ( excludeFact: string | undefined, ): Revision[] => { const { store } = session; - const rows = store.prepare(CAUSE_CHAIN).all({ of, the }) as CauseRow[]; - const revisions = []; - if (rows && rows.length) { - for (const result of rows) { - if (result.fact === excludeFact) { - continue; - } - const revision = getFact(session, { fact: result.fact }); - if (revision) { - revisions.push(revision); + const stmt = store.prepare(CAUSE_CHAIN); + try { + const rows = stmt.all({ of, the }) as CauseRow[]; + const revisions = []; + if (rows && rows.length) { + for (const result of rows) { + if (result.fact === excludeFact) { + continue; + } + const revision = getFact(session, { fact: result.fact }); + if (revision) { + revisions.push(revision); + } } } + return revisions; + } finally { + stmt.finalize(); } - return revisions; }; /** @@ -485,26 +495,31 @@ const getFact = ( { store }: Session, { fact }: { fact: string }, ): Revision | undefined => { - const row = store.prepare(GET_FACT).get({ fact }) as StateRow | undefined; - if (row === undefined) { - return undefined; - } - // It's possible to have more than one matching fact, but since the fact's id - // incorporates its cause chain, we would have to have issued a retraction, - // followed by the same chain of facts. At that point, it really is the same. - // Since `the` and `of` are part of the fact reference, they are also unique. - const revision: Revision = { - the: row.the as MIME, - of: row.of as URI, - cause: row.cause - ? (fromString(row.cause) as Reference) - : refer(unclaimed(row as FactAddress)), - since: row.since, - }; - if (row.is) { - revision.is = JSON.parse(row.is); + const stmt = store.prepare(GET_FACT); + try { + const row = stmt.get({ fact }) as StateRow | undefined; + if (row === undefined) { + return undefined; + } + // It's possible to have more than one matching fact, but since the fact's id + // incorporates its cause chain, we would have to have issued a retraction, + // followed by the same chain of facts. At that point, it really is the same. + // Since `the` and `of` are part of the fact reference, they are also unique. + const revision: Revision = { + the: row.the as MIME, + of: row.of as URI, + cause: row.cause + ? (fromString(row.cause) as Reference) + : refer(unclaimed(row as FactAddress)), + since: row.since, + }; + if (row.is) { + revision.is = JSON.parse(row.is); + } + return revision; + } finally { + stmt.finalize(); } - return revision; }; const select = ( @@ -566,24 +581,30 @@ const toFact = function (row: StateRow): SelectedFact { }; // Select facts matching the selector. Facts are ordered by since. -export const selectFacts = function* ( +export const selectFacts = function ( { store }: Session, { the, of, cause, is, since }: FactSelector, -): Iterable { +): SelectedFact[] { const stmt = store.prepare(EXPORT); - // Note: Cannot finalize() in a generator function's finally block because - // the finally block runs when the generator returns (immediately), not when - // it's exhausted. The statement will be finalized when the store is closed. - for ( - const row of stmt.iter({ - the: the === SelectAllString ? null : the, - of: of === SelectAllString ? null : of, - cause: cause === SelectAllString ? null : cause, - is: is === undefined ? null : {}, - since: since ?? null, - }) as Iterable - ) { - yield toFact(row); + try { + const results = []; + // Note: Cannot finalize() in a generator function's finally block because + // the finally block runs when the generator returns (immediately), not when + // it's exhausted. The statement will be finalized when the store is closed. + for ( + const row of stmt.iter({ + the: the === SelectAllString ? null : the, + of: of === SelectAllString ? null : of, + cause: cause === SelectAllString ? null : cause, + is: is === undefined ? null : {}, + since: since ?? null, + }) as Iterable + ) { + results.push(toFact(row)); + } + return results; + } finally { + stmt.finalize(); } }; @@ -763,9 +784,11 @@ const commit = ( ): Commit => { const the = COMMIT_LOG_TYPE; const of = transaction.sub; - const row = session.store.prepare(EXPORT).get({ the, of }) as + const stmt = session.store.prepare(EXPORT); + const row = stmt.get({ the, of }) as | StateRow | undefined; + stmt.finalize(); const [since, cause] = row ? [ From 960b0c79cfb03c9400697332703067e113040c98 Mon Sep 17 00:00:00 2001 From: Robin McCollum Date: Thu, 30 Oct 2025 15:07:36 -0700 Subject: [PATCH 2/3] Removed comment about finalize, since it's no longer applicable now that I'm not using a generator function. --- packages/memory/space.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/memory/space.ts b/packages/memory/space.ts index 8bca0238b..8df481d20 100644 --- a/packages/memory/space.ts +++ b/packages/memory/space.ts @@ -588,9 +588,6 @@ export const selectFacts = function ( const stmt = store.prepare(EXPORT); try { const results = []; - // Note: Cannot finalize() in a generator function's finally block because - // the finally block runs when the generator returns (immediately), not when - // it's exhausted. The statement will be finalized when the store is closed. for ( const row of stmt.iter({ the: the === SelectAllString ? null : the, From 495631d61c997048b8864ef6f0395e42612776a1 Mon Sep 17 00:00:00 2001 From: cubic Bot Date: Thu, 30 Oct 2025 22:11:50 +0000 Subject: [PATCH 3/3] Always finalize prepared statements by adding try/finally (merges into #1990) --- packages/memory/space.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/memory/space.ts b/packages/memory/space.ts index 8df481d20..f7d92b8bc 100644 --- a/packages/memory/space.ts +++ b/packages/memory/space.ts @@ -782,10 +782,14 @@ const commit = ( const the = COMMIT_LOG_TYPE; const of = transaction.sub; const stmt = session.store.prepare(EXPORT); - const row = stmt.get({ the, of }) as - | StateRow - | undefined; - stmt.finalize(); + let row: StateRow | undefined; + try { + row = stmt.get({ the, of }) as + | StateRow + | undefined; + } finally { + stmt.finalize(); + } const [since, cause] = row ? [