Skip to content

Bug: react-refresh leaks FiberRootNodes from secondary renderers #36379

@xpl

Description

@xpl

React version: 19.2.5

Steps To Reproduce

Repro: https://github.com/xpl/react-leak (npm i && npm run dev)

Reproduces only with react refresh enabled (in dev builds).

The current behavior

Here we have a react-three-fiber tree that is nested inside React context. If the tree is unmounted/mounted repeatedly (with new value creating every time), all previous values are still retained, never getting GC'ed, which creates a severe memory leak if the value is heavy.

        {value ? (
          <Ctx.Provider value={value}>
            <Canvas style={{ width: 200, height: 150 }}>
              <PerspectiveCamera makeDefault />
              <OrbitControls />
            </Canvas>
          </Ctx.Provider>
        ) : (
          <em>unmounted</em>
        )}

In the https://github.com/xpl/react-leak I have added a visualization of GC status for previous values (using WeakRef), so you can observe that they never get collected. You have to press Collect garbage button in DevTools to force collection.

Image

Possible fix

It helps, but I am not sure if it's a nonsense or not.

--- a/packages/react-refresh/src/ReactFreshRuntime.js
+++ b/packages/react-refresh/src/ReactFreshRuntime.js
@@ -518,7 +518,10 @@ export function injectIntoGlobalHook(globalObject: any): void {
     hook.onCommitFiberRoot = function (
       this: mixed,
       id: number,
       root: FiberRoot,
       maybePriorityLevel: mixed,
       didError: boolean,
     ) {
       const helpers = helpersByRendererID.get(id);
       if (helpers !== undefined) {
-        helpersByRoot.set(root, helpers);
 
         const current = root.current;
         const alternate = current.alternate;
@@ -540,6 +543,7 @@ export function injectIntoGlobalHook(globalObject: any): void {
           if (!wasMounted && isMounted) {
             // Mount a new root.
             mountedRoots.add(root);
             failedRoots.delete(root);
+            helpersByRoot.set(root, helpers);
           } else if (wasMounted && isMounted) {
             // Update an existing root.
             // This doesn't affect our mounted root Set.
           } else if (wasMounted && !isMounted) {
             // Unmount an existing root.
             mountedRoots.delete(root);
             if (didError) {
               // We'll remount it on future edits.
               failedRoots.add(root);
             } else {
               helpersByRoot.delete(root);
             }
           } else if (!wasMounted && !isMounted) {
             if (didError) {
-              // We'll remount it on future edits.
-              failedRoots.add(root);
+              // We'll remount it on future edits — but only if we
+              // previously knew about this root. A didError commit
+              // for a root that was never in mountedRoots means the
+              // root was already unmounted and disposed (e.g. a
+              // secondary renderer tearing down). Adding it to
+              // failedRoots would leak the FiberRootNode permanently
+              // since nothing will ever schedule new work on it.
+              if (helpersByRoot.has(root)) {
+                failedRoots.add(root);
+              }
             }
           }
         } else {
           // Mount a new root.
           mountedRoots.add(root);
+          helpersByRoot.set(root, helpers);
         }
       }
 
       // Always call the decorated DevTools hook.
       return oldOnCommitFiberRoot.apply(this, arguments);
     };

Metadata

Metadata

Assignees

No one assigned

    Labels

    Status: UnconfirmedA potential issue that we haven't yet confirmed as a bug

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions