feat(SqlJsRvfBackend): MEMFS safety net via FinalizationRegistry — closes #9 (A, C, D)#10
Merged
Merged
Conversation
…oses #9 (A, C, D) Closes 3 of the 4 enhancements proposed in #9 (downstream consumer feedback from ruvnet/ruflo): A. FinalizationRegistry + leak-detection static (defensive close on GC) ───────────────────────────────────────────────────────────────────── sql.js's Emscripten module is a process-global singleton. Every `new SQL.Database(...)` allocates a MEMFS file (~sizeof database) that only releases on explicit `.close()` — GC of the JS wrapper does NOT reclaim it because `FS.nodes` outlives every wrapper. Downstream report (ruflo#2432): ~36 GB external memory growth over 6 weeks, 203 orphaned 11-MB MEMFS files in a heap snapshot. Fix: module-level FinalizationRegistry that closes the underlying sql.js Database when the wrapping SqlJsRvfBackend gets GC'd without explicit close(). Belt-and-suspenders only — correct consumers (who call close() explicitly) hit no overhead because we unregister on close. Also adds `SqlJsRvfBackend.openCount(): number` static for monitoring dashboards — `external` memory growth correlated with openCount() growth is the signature of the leak class. Caveats (documented in docs/MEMORY-LIFECYCLE.md): - Finalizer timing is non-deterministic; explicit close() remains contract - Finalizer does NOT call db.export() to persist — close() is the only way to guarantee durability - SafetyNet ≠ substitute for the contract C. Integration test: tests/backends/sqljs-memfs-safety.test.ts ───────────────────────────────────────────────────────────────────── 4 assertions: - openCount() increments on initialize() and decrements on close() - double-close is idempotent (no crash, no double-decrement) - load() replaces db handle without leaking the prior - openCount() reflects concurrent backends correctly All 4 pass in 27 ms. Note: we do NOT test that GC actually triggers the finalizer — V8 finalization is non-deterministic and any test would be flaky on CI. Manual GC-forced test left as a `tests/backends/sqljs-memfs-safety.gc.mjs` future addition (runs under `node --expose-gc`). D. docs/MEMORY-LIFECYCLE.md ───────────────────────────────────────────────────────────────────── New doc explaining: - The MEMFS-singleton trap (why heap snapshots show external growth while heapUsed stays flat) - The downstream incident as a worked example - What consumers must do (explicit close, dispose on shutdown, close before replacing in a Map) - The safety net (what FinalizationRegistry does and doesn't cover) - Detection patterns (openCount() in monitoring loops) NOT in this PR — Suggestion B from #9 (typed writeEdges() on controller). That needs cross-cutting design discussion (which controller owns it, how transactions span backends, whether to expose via MCP) — best as a follow-up PR after maintainer triage. Version bump: 3.0.0-alpha.16 → 3.0.0-alpha.17 (PATCH per semver — backward- compatible additions only; existing close() behavior unchanged). Co-Authored-By: RuFlo <ruv@ruv.net>
timothybrush
pushed a commit
to timothybrush/ruflo
that referenced
this pull request
Jun 22, 2026
… 3.13.2 Picks up upstream agentdb fix for the SqlJsRvfBackend MEMFS safety net (ruvnet/agentdb#9, ruvnet/agentdb#10) — FinalizationRegistry now reclaims MEMFS files when a backend wrapper is GC'd without explicit close(). This is the upstream-side fix complementing ruflo's downstream-side fix in v3.13.1 (PR ruvnet#2444): the downstream PR closed prior controllers on replacement; the upstream PR adds a belt-and-suspenders safety net so even consumers who forget close() don't leak. Combined effect: the ruvnet#2432 leak class is closed at BOTH layers. Version bumps: - agentdb 3.0.0-alpha.16 → 3.0.0-alpha.17 (upstream) - @claude-flow/cli 3.13.1 → 3.13.2 (pin bump) - claude-flow umbrella 3.13.1 → 3.13.2 - ruflo wrapper 3.13.1 → 3.13.2 Co-Authored-By: RuFlo <ruv@ruv.net>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What this catches
The downstream incident: ~36 GB external memory growth over 6 weeks in a long-running MCP server. Heap snapshot showed 203 orphaned 11-MB MEMFS files held by sql.js's process-singleton `FS.nodes` array. Root cause was in the consumer (they didn't call `close()` before `Map.set()` replacement), but agentdb couldn't structurally prevent it. This PR adds the safety net.
Test
```
✓ tests/backends/sqljs-memfs-safety.test.ts (4 tests) 27ms
Test Files 1 passed (1)
Tests 4 passed (4)
```
What's NOT in this PR
Suggestion B from #9 (typed `writeEdges()` on controller) needs cross-cutting design discussion — best as a follow-up PR after maintainer triage. Happy to take that on if there's appetite.
Downstream
`ruvnet/ruflo` already shipped its own consumer-side fix (v3.13.1, PR #2444 — close prior controller before `set()`). This PR is the upstream half: even a consumer who forgets the explicit close gets MEMFS reclamation eventually via GC.
🤖 Generated with RuFlo