Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak in Chrome only #12

Open
novacrazy opened this issue Oct 6, 2022 · 4 comments
Open

Memory leak in Chrome only #12

novacrazy opened this issue Oct 6, 2022 · 4 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@novacrazy
Copy link
Member

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 novacrazy added bug Something isn't working help wanted Extra attention is needed labels Oct 6, 2022
@novacrazy
Copy link
Member Author

novacrazy commented Oct 17, 2022

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant