serenity icon indicating copy to clipboard operation
serenity copied to clipboard

LibWeb: Setting .innerHTML leaks the temporary document, temporary root node, and all the tasks created for the temporary document

Open MacDue opened this issue 3 years ago • 1 comments

To reproduce this just set .innerHTML over and over in some kind of loop.

Eventually you'll OOM, or the number of tasks non-runable tasks in the EventLoop will grow so large that just checking for runnable tasks will start taking a large chunk of CPU (10%+).

I attempted to fix this in #14609 which worked for my test sites, but crashed on real sites. I'm not sure how to properly fix this now.

The temporary root node is kept alive my a circular reference from HTMLElement -> DOMStringMap -> HTMLElement, which keeps the document alive. There are also several other strong refs to the document keeping it alive. But it's tricky to untangle with out introducing more crashes.

MacDue avatar Jul 17 '22 17:07 MacDue

Note: NamedNodeMap has a similar issue to DOMStringMap and causes some unfunky unref() crashes. Since a NNRP to the NamedNodeMap is stored on the DOM::Element, but the NamedNodeMap is a RefCountForwarder for the same element.

MacDue avatar Jul 17 '22 17:07 MacDue