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

Potential Memory Leak in Dispatcher #547

Closed
redrockcity opened this issue Jun 2, 2022 · 4 comments
Closed

Potential Memory Leak in Dispatcher #547

redrockcity opened this issue Jun 2, 2022 · 4 comments

Comments

@redrockcity
Copy link

I'm attempting to use Stimulus to implement infinite scroll for a list of results. As the page is scrolled, new pages of results are fetched and their elements added to the DOM. Some of these elements have stimulus controllers, actions, or are targets. As these elements are added to the DOM, Stimulus is correctly attaching controllers and event handlers to them as expected.

As an optimization, as the page is scrolled further, results that have moved off screen are removed from the DOM. Again, Stimulus is correctly detaching controllers and tearing down event handlers as elements are removed.

If the page is scrolled back up to view results that were previously removed from the DOM, they are refetched and reinserted into the DOM. If the page is scrolled up and down repeatedly, result elements will be removed, refetched, and reinserted as needed. The effect is that only the results the user is currently viewing (with a small buffer) are present in the the DOM at any time.

While running some tests of scrolling the page up and down, I noticed the heap size of the page continually growing. I did a heap snapshot and noticed a large amount of detached HTMLElements (representing the removed results) were still on the heap. Although they were not in the DOM, they were still referenced by the eventListenerMaps in the Stimulus dispatcher.

image

Over time, eventListenerMaps is continuing to grow with detached eventTargets even after they have been disconnected from a controller and removed from the DOM. My infinite scroll page certainly exacerbates this effect, however I would think it could still be an issue with long running SPAs or Turbo apps using Stimulus.

I've tried removing the result elements using parentElement.innerHtml = "", element.remove(), parentElement.removeChildren() but am seeing the same effect with eventListenerMaps. Is there a different way of removing elements such that Stimulus will not maintain a reference to them and they can be garbage collected?

Thanks!

@tleish
Copy link
Contributor

tleish commented Jun 7, 2022

Did you happen to read issue #489?

@redrockcity
Copy link
Author

Did you happen to read issue #489?

I did read that issue and attempted to implement the same fix mentioned at the bottom. I looped through all the elements and removed any data-* attributes prior to removing them from the DOM—thinking Stimulus might clean up the event listener mappings to said elements. Unless I did something wrong, it didn't seem to have any effect. I still got an accumulation of detached element references in the eventListenerMaps over prolonged use of the page.

@intrip
Copy link
Contributor

intrip commented Nov 1, 2022

@redrockcity #592 might have fixed this, can you still reproduce using the latest StimulusJS version?

@marcoroth
Copy link
Member

Going to close this as part of #592. @redrockcity please feel free to re-open this one if you are still experiencing this on the latest version of Stimulus. Thank you!

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

No branches or pull requests

4 participants