React: React16 dev memory leak on render with event listeners

[frehner]

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

Using React16 dev, it appears around 8 event listeners are added every time a component is rerendered. They occasionally get garbage collected, but if you're listening to events that happen a lot (scroll, fast typing, etc) it can slow down and kill the tab you're using.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:

https://jsfiddle.net/Luktwrdm/80/
Open dev tools -> performance, check the "Memory" checkbox, and start a performance capture. Then type quickly into the text box for a bit. Stop the performance capture and note the increase in event listeners. Picture included for reference.

What is the expected behavior?

No memory leaks

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

React16, Chrome, MacOS

Let me know if there's anything I can do to help clarify. Hopefully I got the right wording/info in here. Thanks!

[aweary]

Hey @frehner, thanks for the report. I wasn't able to reproduce the memory leak using the JSFiddle you provided; running Chrome 64.0.3282.119 in Incognito Mode I see the listener count remain mostly constant

Did you make sure to run your test in Incognito Mode? It could be a Chrome plugin causing the problem. If you did, can you share any more specific details about your setup (specific Chrome, macOS versions)

Thanks!

[frehner]

@aweary just tried it in incognito (and made sure that all extensions were disabled in incognito), and I get the same thing. Were you typing very fast into the box when you did the recording?

Chrome v63.0.3239.132
MacOS 10.13.2 (17C205)
Mac 15-inch, 2017

[aweary]

I was typing as fast as I could for probably 10-20 seconds. Could you try updating to Chrome v64 and reproducing again?

[frehner]

Updated to 64.0.3282.140, same issue.

Had a coworker try it on his machine and he reproduced it as well.

[gaearon]

I don't believe it has to do with generating events fast. It has to do with renders.

Because fake events are created every time we call into user code in DEV here.

I don't know, can we reuse the same instance somehow?

[frehner]

Ah yeah, renders does appear to be what actually causes it, I was just focusing on how event listeners were being added. I'll update the title to reflect that.

And @aweary I just noticed that your screenshot actually DOES have the listeners being added -- notice the giant cliff on the left side before it appears they get garbage collected or something.

[gaearon]

To be clear they are going to get GC'd. But yes, it would be nice to reduce DEV pressure on GC if we can.

[TheMcMurder]

I was able to reproduce it on both Chrome 63/64

The below screenshot is chrome 64. I'm seeing multiple event listeners being created.

@frehner If I understand correctly this is only happening in Dev mode, so this issue won't affect you in production: https://github.com/facebook/react/blob/885a291141330eb74e8a98316e286a9ad093f22c/packages/shared/invokeGuardedCallback.js#L34

[aweary]

@frehner I don't think that's what it's showing in my case, there's no significant climb in listeners/memory (you can see the numbers at the top stay ~mostly constant) but if others can reproduce then 🤷

[gaearon]

@aweary Are you triggering re-renders?

[frehner]

Are you zoomed out all the way in the timeline?

In this picture of the timeline, I'm zoomed in automatically to the last part. But if you scroll in this area, you can zoom in/out and possibly you're just zoomed into that one area?

[aweary]

@frehner yupp, I'm zoomed all the way out. There's a few jumps in memory usage, but not the steep climb that you're seeing.

@gaearon I'm just smashing keys inside the text input, which is triggering renders. Can you reproduce it too?

Here's a profile I ran in incognito. Listeners stay between 181-185, and memory only moves from 11.7MB to 12.5MB over 30 seconds. If I'm just the exception here for whatever reason, then I'll just go away 😄

[aweary]

@frehner are you still experiencing this issue with the latest 16.3.1 release?

[frehner]

@aweary yup, it appears that it's still doing it. The jsfiddle above was updated to use 16.3.1 and I still see the same behavior as in my previous screenshots.

[kevzettler]

I have also hit this issue. I extracted a reproduction case from a larger project and put it into an isolated create-react-app demo available here:

https://github.com/kevzettler/event-listener-leak

You can trigger the event listener creation by mousing over the blue UI square. This mouse over does not trigger a re-render but does allocate new event-listeners.

Additionally the mouseUp and mouseDown events will trigger a render and additional event-listener allocations

Kevs-MacBook-Pro:event-listener-leak kev$ yarn list react
yarn list v1.4.0
warning Filtering by arguments is deprecated. Please use the pattern option instead.
└─ [email protected]
✨  Done in 1.00s.

Google Chrome is up to date
Version 65.0.3325.181 (Official Build) (64-bit)

MacOS 10.12.5

[gaearon]

@kevzettler Thanks for the repro, but I'm not sure I see a leak on your screenshot. Doesn't it always seem to go back to the baseline after a while?

[kevzettler]

@gaearon yes It baselines after the garbage collector kicks in. It is not a memory leak in the critical failure definition of 'memory leak'. It is a leak in that it's allocating additional event listeners when it is not expected. It appears that this is indicated in the charts @frehner and @TheMcMurder have shared as well.

[gaearon]

As I mentioned before, it is expected that React will allocate a listener on every component render in development mode.

We use fake events to "isolate" component code so that if it throws, the browser displays an "Uncaught error" message even if some code above in the call stack accidentally catches it. Similarly this is what makes "break on all exceptions" work even though React is wrapping your components in a try/catch.

This overhead doesn't exist in production versions.

[serbanghita]

I clearly walked into the that trap, apparently this is mentioned in the docs.

If you’re benchmarking or experiencing performance problems in your React apps, make sure you’re testing with the minified production build.

From https://reactjs.org/docs/optimizing-performance.html

[kevzettler]

Thanks @gaearon, Yes it appears this is dev only and acting as intended. I encountered this while profiling in a dev environment and was concerned. I had not reviewed the optimizing performance docs.

[gaearon]

Seems like it's not a leak, and the growth itself is DEV-only, as expected.

[esromneb]

I was able to reproduce it on both Chrome 63/64

The below screenshot is chrome 64. I'm seeing multiple event listeners being created.

@frehner If I understand correctly this is only happening in Dev mode, so this issue won't affect you in production:

https://github.com/facebook/react/blob/885a291141330eb74e8a98316e286a9ad093f22c/packages/shared/invokeGuardedCallback.js#L34

I don't fully understand the code here, but should the "swapped out" version of invokeGuardedCallback be removed when an element is re-rendered in DEV mode? Seems like this would remove this DEV only bug (which I am currently struggling with)