Memory Leak - React DOM keeps references to stale child components when using the Container/Pure component pattern

[jzworkman]

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.

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.

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

This info was gathered from the follow issue that was marked stale, but is still definitely an issue with Redux and passing store props to Pure Child unconnected components: #16138

[cliffhall]

I’m seeing similar issue but using Hooks / useState and functional components. We have an infinite scroll and a buffer window. After the buffer is full, old components are removed and new ones added, with the size of the buffer being all that should exist in memory at any one time. But memory just keeps rising. There are no setTimer or event listeners or other in app or global objects keeping references to those discarded components, yet they live on.

[gaearon]

I'm a bit confused about the example. You're saying the problem is with using Redux and "pure" components, but then the example uses neither. Can you clarify what you mean by that?

[jzworkman]

@gaearon Sorry this post had two separate examples, the code example posted was from the Original post(that was closed for being stale despite still being an issue). The Redux example is from my personal experience with the same type of memory leak(that has a work around).
IE Redux state passed to a pure unconnected child component from a parent leaks that memory on redux state update/re-render. However, if the child component is also connected to the redux store directly and getting its Props from redux instead of from the Parent, then the garbage collection works correctly and the memory is cleaned up on subsequent renders.

The root of the issue is that when Props/State are updated in the parent component and passed as props to the child component, the child is not correctly disposing of the references to allow garbage collection to occur on future renders.

There is a lot more information in the chain of issue #16138

[gaearon]

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

This is really strongly worded! :D

Yes, it's not ideal, but in some cases we do keep a previous tree in memory — until the next time the component updates. So I'd only say it's a leak if it keeps growing regardless of parent component updates. These tradeoffs could change though (see #16087 for details).

[gaearon]

@cliffhall Your description isn't very helpful without a code sample. What you're describing does sound like a leak but we can't guess what it is unless you reduce it.

[cliffhall]

I realize that, and soon I will attempt to do that. Just saying this behavior isn’t limited to redux and class based components.

…

Sent from my iPhone

On May 4, 2020, at 9:51 AM, Dan Abramov ***@***.***> wrote:  @cliffhall Your description isn't very helpful without a code sample. What you're describing does sound like a leak but we can't guess what it is unless you reduce it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[jzworkman]

Yes, it's not ideal, but in some cases we do keep a previous tree in memory — until the next time the component updates. So I'd only say it's a leak if it keeps growing regardless of parent component updates.

Yea this is the key part we were seeing, that when getting updates to the parent through Redux(we were getting real time updates of a large accounting report via SignalR that would update the parent). This would result in 10+ abandoned trees in the stack, and they were not cleared by forcing a garbage collection cycle.

When I instead hooked the child display components directly up to redux instead of the parent container, then the previous snapshots were correctly garbage collected

[gaearon]

The original code example doesn't repro for me with 16.3.1: https://codesandbox.io/s/dark-silence-ypgum?file=/src/index.js

So we'll need an actual repro case to look into this.

[dmitrysteblyuk]

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

This is really strongly worded! :D

That would be me, sorry!
I can confirm that the original repro doesn't work anymore.
Please check out this new one react-memory-leak-repro.zip

Basically you can modify the list of items (each containing big data) - and whatever you do (removing all, adding then removing etc) there're just still too many stale props in memory.

[EvertEt]

Looking at the repro above (https://codesandbox.io/s/zealous-golick-s7uue?file=/src/index.js), it reminds me of the fix in #15157 regarding stateNode that wasn't ported to #16115 which did get merged.

[cliffhall]

@gaearon I finally tracked down my memory leak. It was occuring in the react-media npm package. So, not related to this thread. My bad.

[jljorgenson18]

Not sure if this helps but we have a bad memory leak related to storing callbacks in Redux that are created in custom hooks. The objects are definitely cleared inside of Redux, but those callbacks being created in the custom hooks seem to hold a reference somewhere and keep the entire render tree around. If I take out the callback, the memory leak goes away. I'm not positive if this is a React problem or a React-Redux problem though.

Nvm, it was the redux logger