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 main library due to event listeners #353

Closed
LazaroOnline opened this issue Jul 29, 2019 · 4 comments
Closed

Memory leak in main library due to event listeners #353

LazaroOnline opened this issue Jul 29, 2019 · 4 comments

Comments

@LazaroOnline
Copy link

Summary

Memory is not being released properly after the "destroy()" method of filePond.

How to reproduce

https://codesandbox.io/embed/frosty-sea-1u5sm
https://1u5sm.csb.app/

Create and destroy instances of FilePond, every time it is destroyed with the method "destroy()" it is not releasing the memory properly. You can profile it with Chrome's devTools using the "Memory" tab, using the "Allocation instrumentation on timeline" option recording while creating/destroying instances, in a playground if you see the memory allocation charts shows the bars still in blue after destroying the filePond instance indicating that the memory is not being garbage collected.

All event listeners should be removed during the execution of the destroy method.

Tested using plain vanilla flavour js with FilePond.js and no plugins (no vue/react/angular...).
There might be more leaks in the plugins and adapters.

Expected behaviour

In above's code sandbox, after clicking in "Toggle Filepond uploader" multiple times, the filepond objects should not have remaining references (especially the event listeners as a common cause), so when the garbage collection occurs, the blue bars become white in Chrome's devTools indicating proper memory disposal.

Additional information

Environment Version
OS Windows 10, but the same will happen in all platforms: MacOS / iOS / Linux / ...
Device Microsoft Laptop, but this bug is device independent.
Browser Chrome 75.0.3770.142 (Official Build) (64-bit)

To solve this please accept the following pull request:
#352
That should fix some memory leaks.

Thank you.

@rikschennink
Copy link
Collaborator

Thanks!

I've inspected the pull request. You can't destroy the painter process as you could have multiple active FilePond instances bound to the same painter (also the paint loop is shared with Doka to group read/writes)

@rikschennink
Copy link
Collaborator

I've applied the fixes to the dropLabel.js and app index.js and that seems to solve the issue, see chart below, this is 2 minutes of creating and destroying FilePond each 2 seconds.

Screenshot 2019-08-01 at 08 45 49

I will decline the pull request because of the painter modifications, those are breaking.

@LazaroOnline
Copy link
Author

Great job! plus you also took care of dropLabel, awesome! the painter object is fine both ways for me, and the memory is not leaked anymore, so problem solved.
Thank you for your fast response and release!

By the way, what memor profiler are you using? I don't recognize it in the screenshot above.

@rikschennink
Copy link
Collaborator

@LazaroOnline Fantastic! This is the Chrome dev tools performance tab.

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

2 participants