Lantern-chat / frontend

Lantern Web Frontend
Other
7 stars 1 forks source link

Memory leak in Chrome only #12

Open novacrazy opened 2 years ago

novacrazy commented 2 years ago

There is a memory leak only in Chrome/Chromium-based wherein detached DOM nodes/Internal Nodes/"Pending Activities" accumulate whenever things are swapped out.

After spending two weeks troubleshooting this myself, I give up. It doesn't make sense.

I know it's partially tied to my "controller" hook system, wherein a parent component passes a callback into a child component for the child to setup callbacks to allow the parent to control it. The parent still owns the child values regardless, not the other way around.

If the callbacks do not use any DOM nodes, the leak doesn't occur. AFAIK there are no cycles/circular references here in JS itself, and the entire controller structure is set to null when the component is cleaned up. Same for Refs. No DOM nodes survive cleanup.

There is a chance that a circular reference is in event-listeners, as the chrome memory devtools point to random event listeners that don't retain references but somehow still show up. The retainer trees it shows don't make any sense.

Even when using untrack to force callers to not register any signals, it makes no difference. It's like using a reference to a DOM node anywhere in an event-listener will cause that entire damn tree to be retained indefinitely, which is absurd.

Firefox shows no such issues. Both the memory devtool and FF's built-in task manager show zero memory leaks over extended testing periods. No extra DOM nodes, everything is perfectly cleaned up.

novacrazy commented 1 year ago

After more research I'm quite confident this is a Chrome bug described here: https://bugs.chromium.org/p/chromium/issues/detail?id=1177010

The timing of this bug appearing coincides with my move away from event-delegation, so as soon as I used real event-listeners on elements it started.

The only fix is either for Chrome to get its shit together or for us to go back to event-delegation. Or adding a banner to use Firefox.

novacrazy commented 1 year ago

I tried to do a custom build of Chromium to switch private InternalNodes to their full names, but it had no effect on the relevant ones, so it's impossible to confirm it's that specific bug.

novacrazy commented 1 year ago

Switched to a hybrid event-delegation setup that avoids all mouse events outside of the ones on document. That did clean up the retainer graph significantly, but did not fix the leak.

Then, I tested SolidJS's involvement by applying a patch recommended by Ryan:

Basically wrap the cleanNode call (in updateComputation)

const value = node.value
cleanNode(node);
node.value = value;

And then in cleanNode set node.value = undefined or something like that.

But that had little affect. It might have removed some retainers within the graph structures in SolidJS, but did not impact the leak. All detached nodes are retained.

So far I have not received any further help from them, so my only conclusion is that Chrome is irredeemably broken. Seriously.

The retainer graphs I have now make no sense, it's all just InternalNode and infinite circular references to everything else, as if the DOM has been entirely inverted and all child nodes retain their parent nodes indefinitely. I can't make sense of any of it. There's nothing that any modern mark-and-sweep GC couldn't figure out, unless it's intentional on Chrome's part.

And again, zero issues in Firefox. Considering adding a banner that says, "This site works best in Firefox due to bugs in Chrome."

Next step is to try flattening the component graph to remove controller patterns.

novacrazy commented 1 year ago

This was also a Chrome bug causing a leak in Lantern: https://bugs.chromium.org/p/chromium/issues/detail?id=1213045