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 - Detached HTML elements #713

Closed
jf-m opened this issue Apr 26, 2019 · 6 comments
Closed

Memory leak - Detached HTML elements #713

jf-m opened this issue Apr 26, 2019 · 6 comments

Comments

@jf-m
Copy link
Contributor

jf-m commented Apr 26, 2019

Expected behavior

I'm using interactjs in a SPA where I need to destroy/recreate several times an interact element.
When I need to destroy an interactable element I do the following:

        interact(element).unset();
        interact.removeDocument(document);

This call should be enough for the GC to collect and garbage all the retained memory by interactjs.

Actual behavior

In the Heap Snapshots of Chrome there is a Detached HTMLElement stored and never released by interactjs every time I unset.
The HTMLElement is referenced in the object Interactable inside the this._latestPointer variable this._latestPointer.eventTarget.

Solving proposition

For the moment, here is what happens inside the unset method:

interaction.stop()

I think we should call a new destroy method from interaction

          if (interaction.interactable === this) {
            interaction.stop()
          }
            interaction.destroy()

Inside the destroy method of interaction, it needs to clear up the memory like so :


    this._latestPointer = {
      pointer: null,
      event: null,
      eventTarget: null
    }

I've tried this solution and its working, the GC can collect and garbage.

I'm not proposing a Pull Request because maybe unset is not the right place for cleaning the memory, and maybe a new destroy or detach method should be provided by scope.

I let you guys decide.

System configuration

interact.js version: 1.4.0-RC.13
Browser name and version: Chrome
Operating System: MacOS

@jf-m
Copy link
Contributor Author

jf-m commented Apr 26, 2019

Seems like _latestPointer is not the only variable that holds a reference to a detached element.
I've detected prevTap from PointerEvent object as well. It also needs to be cleared.

image

@jf-m
Copy link
Contributor Author

jf-m commented Apr 26, 2019

I've created a pull request #715 that solves this issue (at least in my test-case).

Hope it helps!

@jf-m
Copy link
Contributor Author

jf-m commented Apr 26, 2019

Found another one here downPointer in auto-scroll

image

@taye
Copy link
Owner

taye commented Jun 7, 2019

@jf-m Thanks again!

@taye taye closed this as completed Jun 7, 2019
@bpeab
Copy link

bpeab commented Jun 28, 2019

Hello.

Is there any chances that this issue could have been the source of a memory problem causing crashes on Safari iOS ?

@jf-m
Copy link
Contributor Author

jf-m commented Jun 28, 2019

Hi @bpeab,

I would say the answer of your question highly depends on your implementation of interact.js.
This issue covers a case where some detached HTML elements were retained by interact.js in the memory, preventing the GC from collecting it.

This means that the impact of this bug on the performances within your application, (and thus, the possibility of a crash) depends on what is linked to that detached HTML element.

In my case, I've been able to detect this issue because I use interact.js on a fairly big SPA application that uses VueJS. The detached HTML element had some listeners linked to it that referenced one of my heaviest (memory-wise) vueJS component.
Therefore, I was able to see the difference in Chrome Heap snapshots easily (the vue component had a 7 to 12Mb retained size).
I've never tried on Safari, but in my case, it would not surprise me if at some point safari would crash after re-displaying a lot of times that vue component.

Also, this leak happens only when you destroy and re-create the interactable element. So if you don't do it in your application, then this issue is not linked to your crash.

The best way to know the answer would be to use Chrome Heap Snapshots on your application and find out how many memory the detached element takes in your case, and see how it stacks over the time. Even if you are not facing any crashes on Chrome, it would help you anyway I'd say.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants