Form and form control memory leak

[hbgl]

Describe the bug

I have a <form> with one or more Shoelace form controls. When removing a form control or the form itself from the DOM, both the form and the form controls are leaked. To clarify: there are no other references in the application code that keep these objects alive.

Previous issue: #1823

To Reproduce

1. Have a <form> with one or more form controls, e.g. <sl-input>.
2. Remove one of the form controls from the DOM.
3. Observe that the form control is leaked. You can use the Detached Elements tool in Edge or the Memory tool in Chromium based browsers to verify this.
4. Remove all remaining form controls from DOM. Again, observe that they are leaked.
5. Remove the entire <form> from the DOM. It is also leaked.

Note: Before checking for leaked objects, use the dev tools to force a garbage collection.

Demo

https://codepen.io/hbgl/pen/mdoMzXj

Browser / OS

• OS: [Windows 64bit]
• Browser: [Chromium]
• Browser version: [120.0.6099.217]

[cyantree]

I've made a deep dive and am honestly unsure whether this may be a bug in Lit.

Here's an shoelace-free example which shows the same problem:
https://codesandbox.io/p/sandbox/shoelace-memory-issue-7h2hz2?file=%2Findex.html%3A5%2C50

• Add the elements and clear them afterwards. Two elements get created. One which will be correctly garbage collected (it only renders a span) and one which doesn't (it only renders a button)
• Run the garbage collector in the memory tab of Chrome
• Open the console window and type instances. You should get WeakSet {broken-lit-component} which means the one which renders a button didn't get garbage collected.

Because there's no Shoelace involved here, I think that's something for the Lit team.
Which Lit version is bundled in this CDN build? Is there maybe a newer version available for testing?

[claviska]

Thanks for looking into this, @cyantree!

@justinfagnani is this something you might want to look into?

[claviska]

@cyantree We currently ship Lit ^3.0.0 and I believe 3.1.2 is the latest if you want to test that version specifically.

[justinfagnani]

Lit doesn't do anything to the elements though. As far as Lit is concerned, <button> and <span> are just part of template content. We clone the template, then attach the clone to the DOM. When the host is removed we don't do anything because the browser cleans up DOM, and aside from this possible case, is pretty good about cleaning up cycles involving event handlers and such.

Are we sure this isn't a Chrome bug? Did you try this with plain web components?

[cyantree]

Here's an example with plain web components:
https://codesandbox.io/p/sandbox/web-components-leaking-test-t7n4qy?file=%2Findex.html%3A2%2C37

It seems this might be related to ShadowDOM as these elements doesn't seem to be cleaned up nicely.

Here is a test which demonstrates a strange behaviour and might indeed point to a bug in Chromium:

• Add 5 light dom tests, remove all tests and clean the memory. The WeakSet is (correctly) empty.
• Now add 5 shadow dom tests, remove all tests and clean the memory again. Now the WeakSet still (incorrectly) shows the shadow dom tests.
• Now again add 5 light dom tests, remove all tests and clean the memory. Now the WeakSet is empty again, so the shadow dom tests have been removed aswell.

Can you confirm my observation?

[hbgl]

Here's an example with plain web components: https://codesandbox.io/p/sandbox/web-components-leaking-test-t7n4qy?file=%2Findex.html%3A2%2C37

It seems this might be related to ShadowDOM as these elements doesn't seem to be cleaned up nicely.

Here is a test which demonstrates a strange behaviour and might indeed point to a bug in Chromium:

• Add 5 light dom tests, remove all tests and clean the memory. The WeakSet is (correctly) empty.
• Now add 5 shadow dom tests, remove all tests and clean the memory again. Now the WeakSet still (incorrectly) shows the shadow dom tests.
• Now again add 5 light dom tests, remove all tests and clean the memory. Now the WeakSet is empty again, so the shadow dom tests have been removed aswell.

Can you confirm my observation?

I can confirm. I did a bit more testing and the memory leak depends on when the shadow DOM is attached. If you attach it inside the constructor, then there is no leak. If you attach it inside connectedCallback() then the leak occurs.

Lit attaches the shadow DOM inside conenctedCallback():
https://github.com/lit/lit/blob/b779807a82649b5128eb754b77b7b3cb4c7c1f98/packages/reactive-element/src/reactive-element.ts#L1105-L1111

Check out this codepen: https://codepen.io/hbgl/pen/oNVOMWj

[cyantree]

Thanks for checking it out further.

If you create the shadow DOM in the constructor but still append the input element in connectedCallback() the leak still occurs. It only vanishes when you also append the form element in the constructor as you've done in your example.