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 - React DOM keeps references to previous states/props/children of component #16138

Closed
dmitrysteblyuk opened this issue Jul 15, 2019 · 16 comments
Labels
Resolution: Stale Automatically closed due to inactivity Type: Needs Investigation

Comments

@dmitrysteblyuk
Copy link

dmitrysteblyuk commented Jul 15, 2019

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

What is the current behavior?
ReactDOM keeps references to previous states/props/children when component gets updated. All in all consuming three times as much memory as it really needed.

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:

Link to the example below (using production versions of react and react-dom):
https://codesandbox.io/s/epic-bartik-pvgqx.

Consider following example:

import * as React from 'react';
import * as ReactDOM from 'react-dom';

let dataInstanceCount = 0;
class MyBigData {
  constructor() {
    const id = `my-big-data:${dataInstanceCount++}`;
    this.getMyDataId = () => id;
    this.data = new Array(100000).fill('');
  }
}

let componentInstanceCount = 0;
class MyItem extends React.Component {
  constructor(props) {
    super(props);
    this._myItemId = `my-item:${componentInstanceCount++}`;
    this.state = {list: []};
  }

  render() {
    return this.props.item.getMyDataId();
  }
}

class MyApp extends React.Component {
  constructor(props) {
    super(props);
    this.state = {list: []};
  }

  componentDidMount() {
    this.updateList(() => {
      this.updateList(() => {
        this.updateList();
      });
    });
  }

  updateList(callback) {
    this.setState({
      list: [new MyBigData()]
    }, callback);
  }

  render() {
    return this.state.list.map((item) => (
      <MyItem key={item.getMyDataId()} item={item} />
    ));
  }
}

const rootElement = document.getElementById('root');
ReactDOM.render(
  <MyApp />,
  rootElement
);

setTimeout(() => {
  console.log(
    rootElement._reactRootContainer._internalRoot.current.alternate.firstEffect.memoizedProps.item.getMyDataId(),
    rootElement._reactRootContainer._internalRoot.current.alternate.firstEffect.stateNode._myItemId
  );
  // > my-big-data:0, my-item:0
  console.log(
    rootElement._reactRootContainer._internalRoot.current.firstEffect.memoizedProps.item.getMyDataId(),
    rootElement._reactRootContainer._internalRoot.current.firstEffect.stateNode._myItemId
  );
  // > my-big-data:1, my-item:1
  console.log(
    rootElement._reactRootContainer._internalRoot.current.lastEffect.memoizedProps.item.getMyDataId(),
    rootElement._reactRootContainer._internalRoot.current.lastEffect.stateNode._myItemId
  );
  // > my-big-data:2, my-item:2
}, 1000);

I expect only one MyBigObject and one MyItem component to be in the memory. But instead I can see three of each in memory heap snapshot.

UPDATE
As shown in the updated example the references to these objects and components can be accessed in the sub-properties of the root DOM element.

What is the expected behavior?
There's no justifiable reason to keep in memory unmounted components and previous states/props of component after it was updated.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
React 16.9.0, ReactDOM 16.9.0 (Production versions)
Mac/Win

@prabhjyot28
Copy link

Memory leak occurs when, you change the state of an unmounted component, so to remove this kind of warning, you better check is your component is mounted or not before updating its state.
To do this =>

  1. Create a property _isMounted and set it to false inside your components (that have states/Smart Component) :
    _isMounted = false;

  2. Now set it to true, when your component gets mounted & set it to false when it gets unmounted:

componentWillMount()
{
this._isMounted = true;
}

componentWillUnmount()
{
this._isMounted = false;
}

  1. Now every time before updating the state, check whether component is mounted or not, for your case:

    setInterval(() => {
    if(this._isMounted)
    {
    this.setState({
    list: [new MyBigObject()]
    });
    }
    }, 500);

@dmitrysteblyuk
Copy link
Author

Memory leak occurs when, you change the state of an unmounted component, so to remove this kind of warning, you better check is your component is mounted or not before updating its state.

As you can see in the example I'm not unmounting MyApp component at any point.
But there's still a memory leak (multiple instances of MyBigObject are in the memory, despite only one of them is in the state).

@dmitrysteblyuk
Copy link
Author

dmitrysteblyuk commented Jul 24, 2019

Garbage collection is not deterministic.
Probably not, but chrome memory heap snapshot certainly is.

In the updated example I console logged the sub-properties of rood div element to show that the redundant data and components still have references.
Which means they cannot be garbage collected.

@gaearon
Copy link
Collaborator

gaearon commented Aug 15, 2019

Can you verify whether 16.9 solves this?

@dmitrysteblyuk
Copy link
Author

I updated the example link. The external resources now point to react/react-dom 16.9.0 versions.
The issue is still present.

@amrsekilly
Copy link

@gaearon I upgraded both React and ReactDOM to 16.9.0, but it doesn't seem to fix the issue.

@jacky-nirvana
Copy link

I am facing the same problem.

I have a list(for example: list = [10, 20, 30]), use list.map return JSX.Element[],
after the list changed, for example list => [20, 30, 40], the Elements' count are still 3, but fibernode' count is increased, the deleted Element(element of 10)' fibernodes are referenced already.

React & React-Dom: 16.9.0
OS: macOS v10.14.6
Browser: Chrome 76.0

@jzworkman
Copy link

Is there any update on this issue? We are seeing this as a significant issue when using Redux and container components. When our container componet(that is connected to redux store) passes props to the child components, and then the redux store updates. The child component props are being stranded in the dom with old reference(seen in the heap after forcing a garbage collection cycle). This is causing huge amounts of memory bloat when on a page that is connected to signalR for real time collaboration between users(as each redux update creates hundreds of stranded object references in the child components).

I have verified that this is the cause by instead having all of the previously "dumb" pure child components be converted to Connected components and pull their props from redux instead of having the container component pattern control all of the store connections. this then correctly all references the single redux store object and garbage collection works as expected without stranded references.

@BrandonGillis
Copy link

Any news on this issue ?

@shermanrxie
Copy link

Any update on this?

@joaompneves
Copy link

joaompneves commented Nov 21, 2019

I have an app with lots of components that get unmounted all the time and this is a big issue.
I tried to reproduce this problem in a very simple example (https://jsfiddle.net/vne79fzs/), and I'm concerned how easy is to leak memory without developer being aware.
As far as I know the problem occurs at least since 16.0 and is still there (16.12).

Is there any viable workaround? I tried to cleanup the state field, but the memory is hold by other fields.

@sbrichardson
Copy link

sbrichardson commented Nov 23, 2019

I'm seeing this issue also, I noticed because I was working with large CSV/DB files with a Web Worker.

I tried a few different tests and noticed my case was linked to a Detached system/Context from a resize observer that was using useEffect/useState. It was keeping a reference to the state of a nearby component somehow.

It was an Animation test component that I could test for 60fps while parsing a 75MB file using a Web Worker.

// No props, state passed to children
// The local state was in Converter

<PureComponentParent>
  <AnimationFunctionComponent_w_useRef_useEffect_useState />
  <ConverterComponent />
</PureComponentParent>

Screen Shot 2019-11-23 at 2 32 25 PM

I came across this when doing the following:

  1. Upload a large CSV/db file directly to a worker (Pass File obj to Web Worker and read there)
  2. In Worker, export data, convert to JSON, send back to main UI thread
  3. Save to local state in UI Component and terminate the Web Worker
  4. Unmount the component to clear its state (by routing to another page etc).
  5. Route back to the component, loading default empty state.

The previous state (large results) will still be retained after unmount, and can be seen in the heap snapshot from Chrome Dev Tools.

If I hard refresh the browser, the memory will clear as expected

@stale
Copy link

stale bot commented Feb 21, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

@stale
Copy link

stale bot commented Feb 28, 2020

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!

@cliffhall
Copy link

Great that stalebot can close this if the developers simply ignore it long enough. Kudos, React Team!

@jzworkman
Copy link

New issue opened here: #18790

Which has the main content of the original post, plus the description I added in the comments of this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Stale Automatically closed due to inactivity Type: Needs Investigation
Projects
None yet
Development

No branches or pull requests