Skip to content

fix(inspector): release retained snapshot metadata on session cleanup - #141

Merged
steipete merged 3 commits into
mainfrom
fix/inspector-snapshot-cleanup
Oct 8, 2026
Merged

steipete merged 3 commits into
mainfrom
fix/inspector-snapshot-cleanup

Conversation

@steipete

@steipete steipete commented Oct 8, 2026 •

Copy link
Copy Markdown

v8.writeHeapSnapshot() leaves a native snapshot in JavaScriptCore's HeapProfiler. Source inspection shows that in-process HeapProfiler.disable and Session disconnection currently leave this metadata behind, so later collections continue pruning the retained snapshot map.

This change clears the current VM's snapshot metadata through the existing native profiler owner when a local Session disables HeapProfiler or disconnects. It preserves allocation-sampling cleanup and avoids clearing a worker's own VM for an unsupported main-thread connection.

The regression keeps an independent JSC inspector observer connected, writes a real snapshot, and verifies that the observer can resolve a retained marker before cleanup. After cleanup the same lookup must report that no snapshot exists. It covers disable and disconnect with and without prior enable, plus callback completion. No new public API or test-only hook is introduced.

Validation

  • Current head 755b2da222558ba779687722f2615cf4ad49bc48: native CI passes on Linux x64 and macOS arm64. Linux passes 17/17 result rows; macOS passes 13/13. Both pass all 60 inspector-profiler tests and all 24 inspector API tests. CI built synthetic merge 7e9999a849bb29683418502725336e96a9641078.
  • Linux failing-before proof with the current fixture: published Bun 667c4ab22c and fc53bf8c0f each fail all five cleanup cases at the intended final assertion, because the native observer still resolves snapshot metadata after disable/disconnect. Every marker-preview positive control passes. Each invocation filters 55 unrelated cases by test name; no runtime skip is used.
  • Node 24.19.0 on Linux: all five oracle cases pass, including a nonzero snapshot-ID positive control and preservation of the observer's Runtime object after cleanup.
  • Portable Linux build of the exact current head passes all 60 profiler and 24 inspector cases. The dedicated v8-heap-snapshot.test.ts suite passes 7/7 with zero skips. Its first attempt had two setup failures because the declared v8-heapsnapshot@1.3.1 test dependency was absent; installing the declared test workspace dependencies fixed setup without changing source, and the complete snapshot suite then passed.
  • The portable binary's maximum required GLIBC version is 2.17. The maintained Nehalem baseline suite passes 30/30 cases, and an actual stock CentOS 7 container runs the binary and integral allocation workload successfully. Binary SHA256: 6ed43f8a0497032f26cc38e894dc6f77b23b8ca69e2d9e6acc5ec631e3ad8bd4.
  • Independent scoped P2 review, TypeScript formatting, and git diff --check pass.

The initial CI run failed all five new cases because the expected JSC error omitted its data field. It had already resolved the marker before cleanup and received No heap snapshot afterward. A test-only correction now asserts the complete error envelope; the failed run was not rerun or counted as passing.

OpenClaw consumer proof and the shared runtime pin update continue separately. No measured retention improvement or runtime publication is claimed.

Related upstream work

oven-sh/bun#34719 adds broader in-process inspector support but retains stubbed HeapProfiler cleanup. oven-sh/bun#34085 preserves IDs between successive snapshots; it does not implement explicit cleanup. Both remain open. This patch addresses the existing cleanup commands without porting general Runtime evaluation or heap-object-ID APIs.

@steipete
steipete marked this pull request as ready for review October 8, 2026 02:59
@steipete
steipete merged commit 42bd1d2 into main Oct 8, 2026
6 checks passed
@steipete
steipete deleted the fix/inspector-snapshot-cleanup branch October 8, 2026 03:09
steipete added a commit to openclaw/openclaw that referenced this pull request Oct 8, 2026
Advance the shared Bun pin from fc53 to 42bd for deferred VM bytecode generation, integral heap-sampling sizes, and inspector snapshot cleanup (openclaw/bun#137, openclaw/bun#140, openclaw/bun#141). Keep WebKit cb8d6f202b and update the four verified Darwin/Linux artifact pins and CI documentation.

Gate A passed: 35 paired selections, 270 targeted cells, no candidate regressions, ten Node-hidden smoke steps with zero Node attempts, and supplemental JSC checks. Both Darwin architecture gates passed. Preserve the baseline-only UI palette intermittent in #167000; #166780 is not claimed fixed. No new admissions qualified. Pin staging tests, changed checks, independent P2 review, exact-head CI, and ClawSweeper passed without a CI rerun.
steipete added a commit to openclaw/openclaw that referenced this pull request Oct 8, 2026
Use the native profiling owner on Node and Bun while preserving admission, redaction, bounds, and cleanup. Exercise CPU, heap, and snapshot captures on the selected runtime, retain a genuine Node oracle for V8 tracking, and reject Bun environment-started debugger conflicts.

The consumer cutover depends on the shared Bun pin including openclaw/bun#140 and openclaw/bun#141. Node 24 and the combined fixed Bun pass all 112 targeted cases; the previous pin still fails the integral heap-profile contract.
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Oct 9, 2026
Advance the shared Bun pin from fc53 to 42bd for deferred VM bytecode generation, integral heap-sampling sizes, and inspector snapshot cleanup (openclaw/bun#137, openclaw/bun#140, openclaw/bun#141). Keep WebKit cb8d6f202b and update the four verified Darwin/Linux artifact pins and CI documentation.

Gate A passed: 35 paired selections, 270 targeted cells, no candidate regressions, ten Node-hidden smoke steps with zero Node attempts, and supplemental JSC checks. Both Darwin architecture gates passed. Preserve the baseline-only UI palette intermittent in openclaw#167000; openclaw#166780 is not claimed fixed. No new admissions qualified. Pin staging tests, changed checks, independent P2 review, exact-head CI, and ClawSweeper passed without a CI rerun.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant