Frame.removeNode unlinks but never frees Node/Element memory (long-running-page leak)

[staylor]

Summary

Frame.removeNode unlinks a node from its parent and updates ID maps / disconnect callbacks, but never frees the Node (or Element) memory. Across the entire src/browser/ tree there are zero call sites that pass a *Node or *Element to Factory.destroy(). Once a Node is allocated, it lives until the Page is destroyed.

For short-lived browse sessions this is invisible. For long-lived sessions that mutate the DOM repeatedly (virtualized lists, infinite scroll, SPAs with route changes, automation that interacts repeatedly with the same page) memory grows monotonically.

Where it happens

src/browser/Frame.zig:2899 (Frame.removeNode):

pub fn removeNode(self: *Frame, parent: *Node, child: *Node, opts: RemoveNodeOpts) void {
    // ... unlink from parent's children list ...
    const children = parent._children.?;
    switch (children.*) {
        .one => |n| {
            parent._children = null;
            self._factory.destroy(children);  // destroys the Children container
        },
        .list => |list| {
            list.remove(&child._child_link);
            // ...
            self._factory.destroy(list);  // destroys the list container
        },
    }

    child._parent = null;
    child._child_link = .{};

    // ... update live ranges, ID maps, slot assignments ...
    // ... fire disconnectedCallback for custom elements ...
    // ... unhook style sheets if a <style> is being removed ...

    // No call to self._factory.destroy(child) anywhere in this function
    // or in any function it calls. The child Node struct stays allocated.
}

I grepped the whole src/browser/ tree for _factory.destroy(.*[Nn]ode) and _factory.destroy(.*Element): zero hits. The only destroy call sites are for small auxiliary structs (the Children container, MessagePort, CSS prop entries, attributes, iterators, X25519 private keys, WorkerGlobalScope) -- never a Node or Element.

Reproducer (synthetic, no external deps)

const initial = document.querySelectorAll('*').length;

for (let i = 0; i < 10000; i++) {
  const div = document.createElement('div');
  document.body.appendChild(div);
  document.body.removeChild(div);
}

const after = document.querySelectorAll('*').length;
console.log({ initial, after, delta: after - initial });
// Lightpanda: delta is 0 (correct: removed nodes are no longer connected)
// BUT process RSS has grown by ~10000 * sizeof(Element + EventTarget + Node + HtmlElement)

getElementsByTagName('*') only sees connected nodes, so the visible DOM count is correct. The leak is in pmap / RSS, not in DOM-API observability.

Reproducer (realistic, what we hit)

A long-lived Puppeteer + Lightpanda session that drives variant rendering across many product pages (the workload that exposed this for us). After ~50 pages of interactive variant rendering, the Lightpanda process accumulates several hundred MB of Element/Node memory beyond what the live DOM accounts for.

This is somewhat masked by our specific case (sibling issue I'm also filing about CSS-in-JS <style> accumulation) because there the <style> elements stay connected and dominate the symptom. But even on pages without that issue, repeated removeChild will leak.

Suggested fix directions

1. Per-<style>-element destroy hook, narrow scope. Doesn't fix the general leak but catches the case our sibling issue tickles.

2. Track detached subtrees and reclaim them on next macrotask boundary. When removeNode is called and the subtree won't be reattached (opts.will_be_reconnected == false), walk the detached subtree and destroy() each Node + its prototypes. The current code already walks the subtree for disconnectedCallback / ID-map cleanup at lines ~2986-3006 -- adding destroy at the end of that walk is a small change. The catch: any JS-side handle to a detached node would become a use-after-free, so this needs the existing reference-counting (RC(u8)) on Nodes, or a "do not destroy if JS still holds it" check.

3. Per-document arena for Node allocations, reset on full document teardown only. Trades freshness-of-reclaim for simplicity. Not a real fix for long-running pages but bounds the worst case to "one page lifetime".

(2) is the proper fix. (1) is a quick partial mitigation if (2) is far away.

Possibly related

• libdom: unref nodes and strings #80 (closed long ago) was the original libdom-era version of this concern. The DOM has been rewritten in Zig since then, and the current implementation uses RC for some structures (MutationRecord, etc.) but Nodes/Elements aren't on that path.
• Memory regression between nightly.6051 and nightly.6065 — single multi-hundred-MB anon mapping appears #2395 (recent memory regression) was a different specific regression that was fixed; the leak described here is structural rather than a regression.

Environment

• Lightpanda: 1.0.0-nightly.6240+37391687
• Repo HEAD at investigation time: 2f3a426fb0d3
• OS: macOS 14, arm64

Happy to provide a minimal Zig test that exercises the leak (just create + destroy 10k elements and check _factory._slab allocator stats before/after) if that would help land the fix.

[staylor]

Update / scope correction.

The structural finding stands -- Frame.removeNode does unlink without freeing, and _factory.destroy(*Node|*Element) is never called anywhere in src/browser/. That part of the report is independently verifiable from the source and the synthetic 10k-loop reproducer in the body still demonstrates it.

But the "realistic reproducer (what we hit)" section that pointed at long-lived Lightpanda sessions accumulating Element/Node memory beyond what the live DOM accounts for was wrong about cause. After investigating: the Element growth we observed in the production scraper is dominated by <style> elements that stay connected to <head> (not detached), driven by Yotpo widget re-injection on every variant click. That happens identically on real Chromium under the same workload (~193 styles per click on both engines). The reason it shows up asymmetrically in our setup is unrelated: our scraper uses setRequestInterception(true) on Chromium to filter tracking-domain hosts before they ever load, and yotpo.com is on that list. We can't apply the same filter on Lightpanda because of an upstream setRequestInterception + --http-proxy deadlock (separate issue I'll file). So Chromium runs without Yotpo loading; Lightpanda runs with Yotpo loading; the two aren't comparable.

Net: this issue is still valid as a structural finding about removeNode not freeing memory, but the production-impact framing in the body overstates how much it explains what we observed. Realistic test cases that would actually exercise the leak: any synthetic loop that creates and removes elements (the 10k-loop in the body), or applications that legitimately churn DOM via virtualized scroll / route changes / long-lived SPAs.

Happy to defer on priority -- the synthetic reproducer is the part of this issue worth keeping if you want to track the structural fix; if not, close at will.

[karlseguin]

Just to add my $0.02 to this. It's a known/intentional part of the design. We hook into v8's finalizer and pair it with reference counting for a number of non-DOM objects. For example XMLHttpRequest and fetch are both tied into v8's GC this way (and both can pin large memory).

DOM objects can be referenced by many different places, e.g. a NodeList, Event Handlers and Attributes. So we opted to forgo reference counting and rely on the arena. The infrastructure's there to support it, and it gets talked about now and again. It's a tradeoff and for most cases, I think the current model works well.