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

Bug: Memory leak of old state until next render #30727

Open
jtbandes opened this issue Aug 16, 2024 · 2 comments
Open

Bug: Memory leak of old state until next render #30727

jtbandes opened this issue Aug 16, 2024 · 2 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@jtbandes
Copy link

jtbandes commented Aug 16, 2024

React version: 18.3.1

Summary

It appears that state which is no longer being used can still be retained by some __reactFiber$... internals, leading to arbitrarily large memory leaks depending on what other objects are referenced by the state.

Possibly related issues:

Steps To Reproduce

  1. Open the following repro example in Chrome (click Open Preview In New Tab): https://stackblitz.com/edit/react-leak-repro-1?file=src%2FApp.tsx
  2. Open the Chrome Memory inspector
  3. Click the "Mount" button, take a heap snapshot, and verify the LeakyActions object is present
  4. Click the "Unmount" button, take a heap snapshot, and see that LeakyActions is still present
  5. Click the "Increment" button, take a heap snapshot, and verify that LeakyActions is now released.
Screen.Recording.2024-08-16.at.12.08.03.PM.mov

Link to code example:

https://stackblitz.com/edit/react-leak-repro-1?file=src%2FApp.tsx,vite.config.ts

This repro case may be slightly more complicated than necessary, but was reduced from a real-world use case in our app.

import { useImperativeHandle, useState } from 'react';

class LeakyActions {
  foo() {
    console.log('real foo');
  }
}

function WorkspaceAdapter({ r }: { r: React.Ref<LeakyActions> }) {
  useImperativeHandle(r, () => new LeakyActions(), []);
  return null;
}

function App() {
  const [adapterMounted, setAdapterMounted] = useState(false);
  const [actions, setActions] = useState<LeakyActions | null>(null);
  const [counter, setCounter] = useState(0);

  return (
    <>
      <button onClick={() => setAdapterMounted((x) => !x)}>
        {adapterMounted ? 'Unmount' : 'Mount'}
      </button>

      <div>Have actions: {String(actions != undefined)}</div>
      {adapterMounted && <WorkspaceAdapter r={setActions} />}

      <div>Counter: {counter}</div>
      <button onClick={() => setCounter((c) => c + 1)}>Increment</button>
    </>
  );
}

export default App;

The current behavior

The object is retained by the __reactFiber$... property of the <button> element:

image

In our app, this behavior leads to a huge graph of objects from a previous screen/route of the app being retained when switching away from that screen.

The expected behavior

The state which is no longer current should be released.

@jtbandes jtbandes added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Aug 16, 2024
Copy link

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@github-actions github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Nov 14, 2024
@jtbandes
Copy link
Author

I don't think anything has changed 🤷

@github-actions github-actions bot removed the Resolution: Stale Automatically closed due to inactivity label Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

1 participant