When removing a custom component with @Tag, a DetachedHTMLElement occurs

[RhonanMS]

Description

We have found that the RAM usage within the browser increases with each interaction in our applications.
We could trace this back to an "embed" mechanism in our application, where we dynamically add and remove Components (e.g. HorizontalLayout and VerticalLayout) with a number of child components.
During profiling of the code in the browser with several "embed" actions, we found that a lot of DetachedHTMLElements - each corresponding to one of the components removed from the DOM.
We tried to reproduce this in a minimal example (which is attached below), and found that the DetachedHTMLElements seem to occur when we use custom components with an "@tag" annotation. This is even the case, when the custom component is inherited from the Vaadin Div component - as soon as @tag("my-div") was added, the DetachedHTMLElements occur.
Our first thought was that this might have something to do with reusing a detached HTML component, but when the same component is added and removed twice, two instances of the DetachedHTMLElement can be found.

The issue did not seem to be browser related and occured in our standard browser mix: Chrome, Firefox and Edge. Safari was not tested.

Expected outcome

There should not be a DetachedHTMLElement when a custom Component using @tag is used.

Minimal reproducible example

The
attached project contains an example.
It runs on localhost:8181

Steps to reproduce

Note: we use the Edge browser to reproduce the example because of it "Detached Elements" development tool, but the increased memory usage can also be seen in other browsers.

To reproduce

1. set "boolean useDivs" to "true" in EmbedSampleView.
2. run app
3. open Edge Browser in private mode
4. open "Dev Tools"
5. open "Detached Elements" tool
6. reload page if buttons in "Detached Elements" tool are inactive (seems to be the default)
   
7. open http://localhost:8181/
8. click 40 times on "Embed next layout" button
9. open "Detached Elements" tool
10. click "collect garbage" button in "Detached Elements" tool (the trash can icon)
11. click "get detached elements" button in "Detached Elements" tool (the reload icon)
12. there should be around 10 elements (mostly style and one div class="v-reconnect-dialog" element)
    
13. close tab
14. kill app
15. set "boolean useDivs" to "false" in EmbedSampleView.
16. repeat steps 2 to 11
17. there should now be around 40 "vaadin-vertical-layout" elements additionally to the elements described in step 12.
    

Environment

Vaadin version(s): 14.8.13
OS: Windows 10 Enterprise (Build 20H2, OS build 19042.1889)

Browsers

Chrome, Firefox, Edge

[knoobie]

@mshabarov this sounds like something to be taken care in flow

[mshabarov]

Thanks @knoobie ! That might be an issue in core.

[johannest]

Seem to reproduce for me with Edge. In Chrome I used Memory tool and similarly did garbage collect and took a memory snapshot after each 20 click: there the memory consumption increased ~10MB on every iteration, and detached element counts doubled. If using default Divs, Chrome's memory consumption does not increase. So the issue clearly reproduces with Chrome as well.

[johannest]

[mlindfors]

With Firefox, with just Divs, the memory consumption seems to grow to a degree, and then drop down eventually. With my-divs the memory consumption just keeps going up. So I suppose I can confirm that this happens on Firefox as well.

[RhonanMS]

Hello, I just found the issue #13851. Could it be that this bug is a duplicate of the 13851?

[mshabarov]

@RhonanMS we were talking about that on the planning session today, and indeed these two issues might have the same root cause. Note that we have a patch for #13851 and it's under review. Once we get it merged, we'll retest your case.

[RhonanMS]

Hi @mshabarov, that is great news!!! I'm looking forward to the results of the re-test!

[mshabarov]

Vaadin 14.8.18 now released and it contains the patch for this 18c4058 .
To be re-tested with V14.8.18.

[RhonanMS]

Hi @mshabarov,

I re-tested with V14.8.18 (using a clean workspace and emptying the local Maven repository) and unfortunately the problem still seems to exist - both in the example attached in the issue (see also the new screenshot) as well as in our application, the memory consumption increases when custom components are added to the DOM and does not increase when they are removed afterwards

[mcollovati]

Taking a look at a heap dump in Chrome, my guess is that the memory leak may be in Flow client SimpleElementBindingStrategy

    private native void bindPolymerModelProperties(StateNode node,
            Element element)
    /*-{
      if ( @com.vaadin.client.PolymerUtils::isPolymerElement(*)(element) ) {
          this.@SimpleElementBindingStrategy::hookUpPolymerElement(*)(node, element);
      } else if ( @com.vaadin.client.PolymerUtils::mayBePolymerElement(*)(element) ) {
          var self = this;
          try {
              $wnd.customElements.whenDefined(element.localName).then( function () {
                  if ( @com.vaadin.client.PolymerUtils::isPolymerElement(*)(element) ) {
                      self.@SimpleElementBindingStrategy::hookUpPolymerElement(*)(node, element);
                  }
              });
          }
          catch (e) {
              // ignore the exception: the element cannot be a custom element
          }
      }
    }-*/;

Flow registers a callback that captures node and element for the Promise returned by customElements.whenDefined(...), but the promise is never fulfilled.

Looking at the retainers tree, the code relative to the client-xxxxx.cache.js corresponds to the calls to $wnd.customElements.whenDefined(c.localName)

[mcollovati]

Did a quick test, changing the code to attach the callback to a promise that gets fulfilled after a second if the custom element is not registered in the meanwhile, and it seems that with this workaround there are leaks

              var promiseTimeout = new Promise(function(r) { setTimeout(r, 1000) });
              var whenDefinedPromise = $wnd.customElements.whenDefined(element.localName);
              Promise.race([whenDefinedPromise, promiseTimeout])
                .then( function () {
                  if ( @com.vaadin.client.PolymerUtils::isPolymerElement(*)(element) ) {
                      self.@SimpleElementBindingStrategy::hookUpPolymerElement(*)(node, element);
                  }
              });

Before garbage collection

After garbage collection

[vaadin-bot]

This ticket/PR has been released with Vaadin 14.8.19.

[vaadin-bot]

This ticket/PR has been released with Vaadin 23.2.4.

[RhonanMS]

Sorry for not getting back earlier to you, as I was out of order thanks to COVID-19. We retested the issue with both the sample program and our main application and the issue is gone - memory usage in the browser is much better now!
Thanks to everyone involved for your help and work!

[vaadin-bot]

This ticket/PR has been released with Vaadin 23.1.13.

[vaadin-bot]

This ticket/PR has been released with Vaadin 14.9.0.beta1 and is also targeting the upcoming stable 14.9.0 version.