fix: reap all cross-navigation state on warm Page reuse (#33)

[yfedoseev]

Closes #33.

Root cause

The reporter's diagnosis was right about the mechanism but incomplete about the sources. The leak isn't one thing: every reaper the engine had was wired only to Page::drop — which a pool by definition never reaches — and several bootstrap-JS registries are scoped to the JsRuntime rather than to the document. On the cold path that distinction never mattered, because the runtime is the page. On the warm path replace_dom swaps the document underneath them and they accumulate forever.

Everything below was unpruned on reuse:

Registry	File	Why it retains
_objListeners	event_bootstrap.js:298	WeakMap keyed by target object — but window-bound listeners are keyed against the one object that is never collected for the life of the isolate. Their closures pin the page's whole object graph.
_nodeListeners	event_bootstrap.js:297	Strong Map, never pruned at all. The dominant source.
_nodeCache, _scrollState	dom_bootstrap.js:5-6	Strong Maps keyed by nodeId. The WeakRef values don't help — an old wrapper stays alive as long as any listener closure references it.
_moObservers	dom_bootstrap.js:1926	Only shrinks on disconnect(), which pages rarely call.
_appendedIframes, _frameRegistry	dom_bootstrap.js:2034, 2395	Iframe element wrappers and child realm windows.
_customElementsRegistry, _whenDefinedPromises	window_bootstrap.js:4579-4580	Page-supplied constructors, forever.
globalThis properties	—	window.__APP_STATE = …, framework singletons. The reporter suspected this; it's real.
on* handler values	—	window.onscroll = fn. These already exist as own properties at bootstrap, so a key-set diff cannot see the assignment.

Confirming the two things the report ruled out: __cancelAllTimers() is present on the warm path, and drain_owned_workers was already called from reset_warm_state on main (page.rs:1560) — so suggestion #2 was partly landed already.

Changes

New JS reset hooks, all non-enumerable so they don't widen Object.getOwnPropertyNames(window):

• __cancelAllListeners() — event_bootstrap.js (the ticket's suggestion feat: profile-driven Client Hints + canonical plugins/mimeTypes rewrite #1)
• __resetDomRegistries() — dom_bootstrap.js
• __resetCustomElements() — window_bootstrap.js
• __markGlobalsBaseline() / __resetPageGlobals() — cleanup_bootstrap.js

Bundled behind a new public Page::reset_for_reuse() (the ticket's suggestion #2). Called by PagePool::acquire, Page::navigate_warm, and the CDP protocol server.

I did not take suggestion #3 (create_realm() per reuse) — it would discard most of the pooling benefit, and the numbers below show it isn't needed.

The globals sweep is a diff, not a wipe

__resetPageGlobals() deletes only keys added since a baseline the engine marks after installing its post-bootstrap instrumentation and before any page script runs. on* handlers get value-level treatment: values are snapshotted at baseline and restored, which clears window.onscroll = fn while preserving the engine's own window.onerror instrumentation — that one is installed once on the cold build and never re-applied on the warm path, so a blanket wipe would have silently disabled error capture for every pooled page after the first. There's a dedicated test for exactly this.

Evidence: A/B on live heap

reload_html without reset_for_reuse is precisely what 0.1.0's PagePool::acquire did, so the control arm reproduces 0.1.0 behaviour and the other arm is HEAD. Comparing them in one build is more meaningful than comparing against a 0.1.0 checkout, which has no heap-inspection API to measure with in the first place.

25 warm reuses of a document retaining ~1 MB behind a window listener closure. Measured after a forced full GC, so these are live/reachable objects — which is the crux: the reporter observed a full GC recovered only 1–2%, identifying the growth as live references rather than deferred garbage.

Arm	Live heap growth per reuse
Without reset (= 0.1.0)	1,040,662 B (~1.04 MB)
With reset (this PR)	468 B

≈2,200× reduction; the control arm recovers essentially the full ~1 MB the document retains, matching the reporter's ~10 MB/page on real product pages scaled to this synthetic doc.

Both arms are committed as tests/warm_reuse_heap_growth.rs. The control arm asserts it does leak — if it ever stops, the positive test has gone vacuous and should fail loudly rather than silently prove nothing.

Correctness bugs fixed by the same change

Two of these are not memory issues and are worth calling out separately:

1. The previous page's handlers fired on the new document. _nodeListeners and _nodeCache are keyed by nodeId, and node IDs restart at zero when replace_dom swaps the document — so the old page's listener for node 42 fired on the new page's node 42, and the new page's node could be handed the old page's wrapper with its expandos.
2. Custom elements could not be re-defined across a warm navigation. customElements.define() for a name the previous page registered was a silent no-op, so the new page's class never upgraded.
3. __keepLongTimersRefed stayed set after a challenge page, pinning long timers on every later navigation of that Page.
4. The CDP protocol server had the identical leak. Page.navigate swaps the document via reload_html on a Page the session keeps alive for its whole lifetime.

Also added

Page::v8_heap_used_bytes() and Page::collect_garbage() (also on BrowserJsRuntime), so operators can verify pool health directly — sample after each navigation and a healthy pool stays flat. The A/B test is built on them.

Removed

Dead _listeners registry in event_bootstrap.js — declared, never read.

Testing

11 new tests across two files, all passing locally (release, --test-threads=1):

tests/warm_reuse_reset.rs ......... 9 passed
tests/warm_reuse_heap_growth.rs ... 2 passed

Covering: window-listener unbinding, node-listener cross-document misfire, page-global sweep, document.on* handlers, engine on* handlers preserved, custom-element redefinition, timers still functional after reset, idempotency across 10 reuses, a guard that the sweep does not strip engine globals (document/fetch/setTimeout/… still defined and the DOM still usable), plus both A/B arms.

cargo clippy --lib -- -D warnings and cargo fmt --all --check are clean.

Note: tests/debug_blocked.rs has a pre-existing clippy failure (redundant reference in println!) under newer clippy, untouched by this PR — it will trip --all-targets independently of these changes.

Bindings

Checked: neither browser_oxide_py nor browser_oxide_mcp exposes PagePool or warm reuse, so there is no binding surface to propagate this to. The one other in-tree warm-reuse consumer was the CDP server, fixed here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Ptn8MHsL8gwcsszwquKrfu

[yfedoseev]

Real-site regression: main → HEAD

Ran the sweep_metrics harness (the same one canary.yml uses) against 15 open, unprotected sites, on both engine paths, from a main worktree and from this branch on the same machine and network. This covers the dependency bumps as well as the #33 fix.

Cold path (Page::navigate)

12/15 both sides. Byte-identical output on every site except hackernews (34,772 → 34,837, dynamic front page). No regressions — meaning deno_core 0.403→0.408, taffy 0.8→0.11 and sha1/sha2 0.10→0.11 are behaviour-neutral on real content.

Pool path (PagePool, warm reuse) — 11/15 → 12/15

site	main	HEAD
hackernews	THIN-BODY len=9	L3-RENDERED len=34,772	fixed
other 14	—	identical	no change

On main, the second site through the pool renders a 9-byte body. Reproduced deterministically — two runs per side, 9, 9 on main and 34772, 34772 on HEAD. hackernews is the first warm reuse in the corpus (wikipedia is site 1), so this is the previous document's state contaminating the next one: exactly the class of bug this PR fixes, showing up on a real site rather than a synthetic one.

That also means the leak wasn't only a memory problem — warm reuse was silently producing wrong output, and the 15 KB canary threshold wouldn't have caught it because canary.yml only exercises the cold path.

Note on the three "both fail" sites

apache (10,485 B), crates_io (4,997 B) and nginx (13,694 B) are L3-RENDERED on both sides — they simply fall under the harness's arbitrary 15 KB "production" threshold because the pages are small. Unchanged between main and HEAD, so not regressions.

Reproduce

cargo build --release -p browser_oxide --example sweep_metrics
BROWSER_OXIDE_SWEEP_POOL=1 \
  target/release/examples/sweep_metrics chrome_148_macos corpus.json out.json

Full per-site table

COLD PATH  (Page::navigate)  main -> HEAD
site             main tag        main len  HEAD tag        HEAD len  verdict
apache           L3-RENDERED        10485  L3-RENDERED        10485  both under 15K
crates_io        L3-RENDERED         4997  L3-RENDERED         4997  both under 15K
docs_rs          L3-RENDERED        27974  L3-RENDERED        27974  same
gnu              L3-RENDERED      1028663  L3-RENDERED      1028663  same
go_dev           L3-RENDERED        47464  L3-RENDERED        47464  same
hackernews       L3-RENDERED        34772  L3-RENDERED        34837  same
kernel_docs      L3-RENDERED        16721  L3-RENDERED        16721  same
mdn              L3-RENDERED       184209  L3-RENDERED       184209  same
nginx            L3-RENDERED        13694  L3-RENDERED        13694  both under 15K
python_docs      L3-RENDERED       112662  L3-RENDERED       112662  same
python_org       L3-RENDERED        62413  L3-RENDERED        62413  same
rustlang         L3-RENDERED        18417  L3-RENDERED        18417  same
w3c              L3-RENDERED       166131  L3-RENDERED       166131  same
wikipedia        L3-RENDERED       259433  L3-RENDERED       259433  same
wikipedia_main   L3-RENDERED       259665  L3-RENDERED       259665  same
  main pass: 12/15   HEAD pass: 12/15

POOL PATH  (PagePool warm reuse)  main -> HEAD
hackernews       THIN-BODY              9  L3-RENDERED        34837  improved
(all 14 others identical)
  main pass: 11/15   HEAD pass: 12/15

TOTAL REGRESSIONS: none

Unit tests

597 passed; 1 failed on Windows — the single failure is perf_ext::tests::distribution_has_distinct_jitter_values, which is pre-existing and environment-dependent, not a regression:

• perf_ext.rs is untouched by this branch (git diff main HEAD is empty for it).
• It fails identically on the pre-dependency commit.
• Test (ubuntu-latest, nightly) passes on this PR, so it's Windows-specific.

Mechanically: with a coarse Windows timer, q = floor(raw_us/100)*100 stays constant across the tight 500-iteration loop, so the monotonic clamp candidate.max(last_us) only advances on a new running maximum — roughly 6 distinct values against a >10 assertion. Worth a separate issue to make the test clock-granularity-aware; out of scope here.

[yfedoseev]

Superseded by #35. GitHub auto-closed this PR when the branch was renamed release/0.1.1 -> release/0.1.2 (0.1.1 had already shipped as a bugfix), and a closed PR whose head ref no longer exists can't be reopened. #35 carries this work forward plus the dependency refresh, security fixes and the real-site regression run. The analysis in this thread still applies.