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

CHANGELOG.md

@@ -6,6 +6,61 @@ follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
 
+## [0.1.1]
+
+### Fixed
+- **`PagePool` / warm reuse leaked V8 heap without bound**
+  ([#33](https://github.com/yfedoseev/browser_oxide/issues/33)). Reusing a
+  `Page` across navigations grew V8's live (non-collectable) heap by ~10 MB
+  per page, eventually OOMing long batches. Every one of the engine's reapers
+  was wired only to `Page::drop`, which a pool by definition never reaches,
+  and the bootstrap JS keeps several registries scoped to the `JsRuntime`
+  rather than to the document. Now reaped on reuse:
+  - all registered event listeners (`__cancelAllListeners()` in
+    `event_bootstrap.js`) — `window`-bound listeners were keyed against the
+    one object that outlives every navigation, so their closures pinned the
+    previous page's entire object graph, and `_nodeListeners` was a strong
+    `Map` that was never pruned at all;
+  - the DOM node-wrapper cache, scroll state, `MutationObserver` registry,
+    and iframe/frame registries (`__resetDomRegistries()`);
+  - custom-element definitions (`__resetCustomElements()`);
+  - globals the page hung off `window` (`__resetPageGlobals()`), diffed
+    against a baseline the engine marks before any page script runs.
+- **Warm reuse misfired the previous page's handlers on the new document.**
+  `_nodeListeners` and the node-wrapper cache 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).
+  Fixed by the same reset.
+- **Custom elements could not be re-defined across a warm navigation.**
+  `customElements.define()` for a name the *previous* page had registered was
+  a silent no-op, so the new page's class never upgraded.
+- `Page::navigate_warm` left `__keepLongTimersRefed` set after a challenge
+  page, pinning long timers on every subsequent navigation of that `Page`.
+- **The CDP protocol server leaked the same way.** `Page.navigate` swaps the
+  document with `reload_html` on a `Page` the session keeps alive for its
+  whole lifetime, so it accumulated the previous document's state for as long
+  as a client stayed connected. It now resets between documents.
+- Page-assigned `on*` handlers (`window.onscroll = …`, `document.onclick = …`)
+  survived reuse. These already exist as own properties at bootstrap, so a
+  key-set diff cannot see the assignment; handler *values* are now snapshotted
+  at baseline and restored, which clears page assignments while preserving the
+  engine's own `window.onerror` instrumentation.
+
+### Added
+- `Page::reset_for_reuse()` — public, bundles every cross-navigation reaper
+  (timers, listeners, DOM registries, custom elements, page globals, orphan
+  Workers, child iframe isolates). Consumers that hand-roll page reuse — e.g.
+  calling `Page::reload_html` on a `Page` they keep alive — should call this
+  between documents; `PagePool`, `Page::navigate_warm` and the CDP server
+  already do.
+- `Page::v8_heap_used_bytes()` and `Page::collect_garbage()` (also on
+  `BrowserJsRuntime`) — lets pool operators verify heap health directly.
+  Sample after each navigation; a healthy pool stays flat.
+
+### Removed
+- Dead `_listeners` registry in `event_bootstrap.js` (declared, never read).
+
 ## [0.1.0] — 2026-06-13
 
 > First open-source release of BrowserOxide — a from-scratch stealth headless

Cargo.toml

@@ -8,7 +8,7 @@ members = ["crates/browser_oxide", "crates/browser_oxide_mcp"]
 exclude = ["crates/browser_oxide_py"]
 
 [workspace.package]
-version = "0.1.0"
+version = "0.1.1"
 edition = "2021"
 license = "MIT OR Apache-2.0"
 authors = ["Yury Fedoseev"]
@@ -39,7 +39,7 @@ doc_overindented_list_items = "allow"
 # The single engine crate — depended on by browser_oxide_mcp. Explicit
 # version pinned to the workspace version so cargo-deny's `wildcards =
 # "deny"` rule doesn't trip on path-only `version = "*"` resolution.
-browser_oxide = { version = "0.1.0", path = "crates/browser_oxide" }
+browser_oxide = { version = "0.1.1", path = "crates/browser_oxide" }
 
 # Async runtime + common derive deps shared by multiple crates.
 tokio = { version = "1", features = ["full"] }

crates/browser_oxide/src/js_runtime/js/cleanup_bootstrap.js

@@ -633,6 +633,123 @@
         internals.push('SharedArrayBuffer');
     }
 
+    // -- Warm-reuse global-namespace reset ---------------------------
+    // The last retention source for a pooled `Page`: properties page
+    // scripts hang straight off the global (`window.__APP_STATE = …`,
+    // `window.onscroll = …`, framework singletons). `globalThis` is the
+    // same object for the whole life of the `JsRuntime`, so on the warm
+    // path every one of those — and everything they transitively
+    // reference — survives into the next navigation. A real browser gives
+    // each navigation a fresh global; this is the closest equivalent that
+    // keeps the expensive bootstrap intact.
+    //
+    // `__markGlobalsBaseline()` snapshots the engine-owned key set;
+    // `__resetPageGlobals()` deletes everything added since. Rust re-marks
+    // the baseline once more after it installs the post-bootstrap
+    // instrumentation (`__cookieWrites` / `__scriptErrors` / the fetch +
+    // XHR wrappers), which is why those names are also allowlisted below —
+    // construction paths that skip the re-mark must not lose them.
+    // Note `window === globalThis` here (dom_bootstrap.js), so scrubbing
+    // the global object covers both.
+    // Guarded: this file is executed TWICE per page — once from
+    // `BrowserJsRuntime`'s constructor (before any page script) and again
+    // from `build_page_with_scripts_*` after the document's scripts have
+    // run. Only the first execution may seed the baseline; re-running the
+    // definitions would also reset the closure variable and throw the real
+    // baseline away.
+    if (typeof globalThis.__resetPageGlobals !== 'function') {
+        let _globalsBaseline = null;
+        let _onHandlerBaseline = null;
+        const _BASELINE_ALWAYS = [
+            '_browser_oxide', '__cookieWrites', '__scriptErrors',
+            '__bo_input_events', '__jsCookies',
+        ];
+
+        // `on*` handlers need value-level treatment, not just key-level.
+        // `onscroll`, `onerror`, … already EXIST as own properties of the
+        // global at bootstrap (default `null`), so a page that assigns
+        // `window.onscroll = fn` mutates a baseline key rather than adding
+        // one — the key-set diff below cannot see it, and the closure (plus
+        // everything it captures) survives the navigation.
+        //
+        // Blanket-nulling them is wrong: the engine itself installs
+        // `window.onerror` as its script-error instrumentation, once, and
+        // does NOT re-install it on the warm path. So snapshot the values
+        // at baseline and RESTORE them, which nulls page assignments while
+        // preserving the engine's.
+        const _snapshotOnHandlers = (target) => {
+            const m = new Map();
+            if (!target) return m;
+            let names;
+            try { names = Object.getOwnPropertyNames(target); } catch (_e) { return m; }
+            for (const k of names) {
+                if (!k.startsWith('on')) continue;
+                try { m.set(k, target[k]); } catch (_e) {}
+            }
+            return m;
+        };
+        const _restoreOnHandlers = (target, baseline) => {
+            if (!target || !baseline) return;
+            let names;
+            try { names = Object.getOwnPropertyNames(target); } catch (_e) { return; }
+            for (const k of names) {
+                if (!k.startsWith('on')) continue;
+                try {
+                    if (typeof target[k] !== 'function') continue;
+                    const orig = baseline.get(k);
+                    // Already the engine's own handler ⇒ leave it alone.
+                    if (orig === target[k]) continue;
+                    target[k] = (typeof orig === 'function') ? orig : null;
+                } catch (_e) {}
+            }
+        };
+
+        Object.defineProperty(globalThis, '__markGlobalsBaseline', {
+            value: function __markGlobalsBaseline() {
+                const seen = new Set(_BASELINE_ALWAYS);
+                for (const k of Object.getOwnPropertyNames(globalThis)) seen.add(k);
+                for (const s of Object.getOwnPropertySymbols(globalThis)) seen.add(s);
+                _globalsBaseline = seen;
+                // `document` is a singleton that survives `replace_dom`, so
+                // `document.onclick = fn` persists exactly like the window
+                // case and needs the same treatment.
+                _onHandlerBaseline = {
+                    global: _snapshotOnHandlers(globalThis),
+                    document: _snapshotOnHandlers(globalThis.document),
+                };
+            },
+            writable: true, configurable: true, enumerable: false,
+        });
+        Object.defineProperty(globalThis, '__resetPageGlobals', {
+            value: function __resetPageGlobals() {
+                // No baseline ⇒ nothing to compare against; deleting on a
+                // guess would strip the engine's own globals.
+                if (!_globalsBaseline) return 0;
+                let removed = 0;
+                const keys = Object.getOwnPropertyNames(globalThis)
+                    .concat(Object.getOwnPropertySymbols(globalThis));
+                for (const k of keys) {
+                    if (_globalsBaseline.has(k)) continue;
+                    // Best-effort: a page can install a non-configurable
+                    // property, and `delete` cannot remove those.
+                    try { if (delete globalThis[k]) removed++; } catch (_e) {}
+                }
+                if (_onHandlerBaseline) {
+                    _restoreOnHandlers(globalThis, _onHandlerBaseline.global);
+                    _restoreOnHandlers(globalThis.document, _onHandlerBaseline.document);
+                }
+                return removed;
+            },
+            writable: true, configurable: true, enumerable: false,
+        });
+        // Seed the baseline on this first execution: it runs as the last
+        // bootstrap, before anything page-authored, so the global namespace
+        // is exactly the engine's. Rust re-marks once more after installing
+        // the post-bootstrap instrumentation. The `internals` purge below
+        // only ever REMOVES keys, so marking before it is safe.
+        globalThis.__markGlobalsBaseline();
+    }
+
     for (const name of internals) {
         [globalThis, globalThis.window].forEach(obj => {
             if (!obj || !(name in obj)) return;
@@ -648,4 +765,5 @@
             }
         });
     }
+
 })(globalThis);

crates/browser_oxide/src/js_runtime/js/dom_bootstrap.js

@@ -3349,4 +3349,40 @@
         configurable: true,
         writable: false,
     });
+
+    // Warm-reuse DOM-registry reaper. Every registry below is module-private
+    // and keyed by (or holding) state that belongs to ONE document, yet it
+    // lives as long as the `JsRuntime`. On the cold path that is exactly the
+    // life of the page, so nothing was ever pruned; on the warm path
+    // (`PagePool` / `Page::navigate_warm`) `replace_dom` swaps the document
+    // underneath them and they accumulate forever. See
+    // `Page::reset_for_reuse`, which calls this.
+    //
+    // `_nodeCache` is doubly wrong across a swap: it is keyed by `nodeId`, and
+    // node IDs restart at zero for the new document, so a surviving entry
+    // hands the NEW page's node the OLD page's wrapper (with the old page's
+    // expandos on it). The `WeakRef` values do not save us — an old wrapper
+    // stays alive as long as any listener closure references it.
+    Object.defineProperty(globalThis, '__resetDomRegistries', {
+        value: function __resetDomRegistries() {
+            _nodeCache.clear();
+            _scrollState.clear();
+            _syncFetchInFlight.clear();
+            // Observers registered by the previous page's scripts. Pages
+            // routinely never call `disconnect()`, so this only shrinks on
+            // reuse — each retained observer pins its callback closure and
+            // every observed target wrapper.
+            _moObservers.length = 0;
+            _appendedIframes.length = 0;
+            _frameRegistry.length = 0;
+            try { globalThis.__ifAppendCount = 0; } catch (_) {}
+            // Re-seed the document wrapper: `_wrapNode` must keep returning
+            // the singleton `_document` for the document node id, which
+            // `replace_dom` preserves.
+            try { _nodeCache.set(ops.op_dom_document_node(), new WeakRef(_document)); } catch (_) {}
+        },
+        writable: true,
+        configurable: true,
+        enumerable: false,
+    });
 })(globalThis);

crates/browser_oxide/src/js_runtime/js/event_bootstrap.js

@@ -1,5 +1,4 @@
 ((globalThis) => {
-    const _listeners = new Map(); // nodeId → Map<eventType, [{callback, capture, once}]>
     // ---- Trusted-event authenticity (v0.1.0 behavioral E1) ----------------
     // `isTrusted` MUST be both unforgeable and shaped like a real browser's:
     //   * a GETTER on Event.prototype — NOT an own data property. Scripts
@@ -295,7 +294,40 @@
 
     // --- EventTarget core logic ---
     const _nodeListeners = new Map(); // nodeId → Map<eventType, [{callback, capture, once}]>
-    const _objListeners = new WeakMap(); // object → Map<eventType, [{callback, capture, once}]>
+    let _objListeners = new WeakMap(); // object → Map<eventType, [{callback, capture, once}]>
+
+    // Warm-reuse listener reaper — the events-side analogue of
+    // `timer_bootstrap.js`'s `__cancelAllTimers()`. A pooled `Page`
+    // (`PagePool` / `Page::navigate_warm`) keeps ONE `JsRuntime` alive across
+    // navigations, so both registries above outlive the document they were
+    // populated for. Two distinct failures follow:
+    //
+    //   * Leak. `_objListeners` is keyed by target *object*; listeners a page
+    //     attaches to `window`/`globalThis` (analytics, scroll handlers, …)
+    //     are keyed against the one global that is never collected for the
+    //     life of the isolate, so those callbacks — and every closure
+    //     variable they capture, which can be the page's whole object graph —
+    //     are retained forever. `_nodeListeners` is worse: it is a *strong*
+    //     Map that is never pruned at all. Measured at ~10 MB/page of live
+    //     (non-GC-able) V8 heap on real product pages, unbounded.
+    //   * Cross-page misfire. `_nodeListeners` is keyed by `nodeId`, and node
+    //     IDs restart from zero when `replace_dom` swaps the document. The
+    //     previous page's handler for node 42 therefore fires on the *new*
+    //     page's node 42.
+    //
+    // Called from `Page::reset_for_reuse` alongside `__cancelAllTimers()`.
+    // Non-enumerable so it does not widen `Object.getOwnPropertyNames(window)`.
+    Object.defineProperty(globalThis, '__cancelAllListeners', {
+        value: function __cancelAllListeners() {
+            _nodeListeners.clear();
+            // Reassign rather than clear: WeakMap has no `clear()`, and the
+            // whole point is to drop the `window`-keyed entry.
+            _objListeners = new WeakMap();
+        },
+        writable: true,
+        configurable: true,
+        enumerable: false,
+    });
 
     const _getNodeIdOrMinusOne = (globalThis.__browser_oxide && globalThis.__browser_oxide._getNodeId)
         ? globalThis.__browser_oxide._getNodeId

crates/browser_oxide/src/js_runtime/js/window_bootstrap.js

@@ -7081,4 +7081,21 @@
     for (let i = 0; i < 5; i++) _defineIframeGetter(i);
 
     Object.defineProperty(globalThis, Symbol.toStringTag, { value: "Window", configurable: true });
+
+    // Warm-reuse custom-element reaper. Both registries hold page-supplied
+    // constructors (and, for `whenDefined`, unresolved promise resolvers)
+    // for the life of the `JsRuntime`, so on a pooled `Page` they retain
+    // every class every previously-loaded document ever defined. Clearing
+    // also fixes a correctness bug: re-`define()`ing a name the *previous*
+    // page had already registered is a no-op today, so the new page's
+    // element class never upgrades. Called by `Page::reset_for_reuse`.
+    Object.defineProperty(globalThis, '__resetCustomElements', {
+        value: function __resetCustomElements() {
+            _customElementsRegistry.clear();
+            _whenDefinedPromises.clear();
+        },
+        writable: true,
+        configurable: true,
+        enumerable: false,
+    });
 })(globalThis);

crates/browser_oxide/src/js_runtime/mod.rs

@@ -130,6 +130,31 @@ impl BrowserJsRuntime {
         self.inner.v8_isolate().cancel_terminate_execution();
     }
 
+    /// V8's `used_heap_size` for this isolate, in bytes.
+    ///
+    /// Intended for monitoring warm reuse: pair with [`Self::collect_garbage`]
+    /// and sample after each navigation. On a healthy pool the value is flat
+    /// across navigations; a monotonic climb means something is retaining the
+    /// previous page (see `Page::reset_for_reuse`).
+    ///
+    /// Note this is V8 heap only — it excludes external/`ArrayBuffer` backing
+    /// stores and everything Rust-side, so it is not process RSS.
+    pub fn v8_heap_used_bytes(&mut self) -> usize {
+        self.inner.v8_isolate().get_heap_statistics().used_heap_size()
+    }
+
+    /// Ask V8 to perform a full garbage collection.
+    ///
+    /// Only meaningful for measurement: call it before
+    /// [`Self::v8_heap_used_bytes`] so the reading reflects *live* (reachable)
+    /// objects rather than not-yet-collected garbage. Without it, heap-growth
+    /// numbers are dominated by GC scheduling noise. Not a correctness tool —
+    /// never call it on a hot path.
+    pub fn collect_garbage(&mut self) {
+        let _guard = IsolateEnterGuard::enter(self.inner.v8_isolate());
+        self.inner.v8_isolate().low_memory_notification();
+    }
+
     /// Execute a JavaScript script and return the string representation of the result.
     ///
     /// Uses V8 directly in a single HandleScope — avoids the overhead of