Fix fiber memory leak with runAllPassiveEffectDestroysBeforeCreates (#…

…18554)
facebook · Apr 9, 2020 · 0566b70 · 0566b70
1 parent ca1a703
commit 0566b70
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 22 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -2368,12 +2368,15 @@ function flushPassiveEffectsImpl() {
         }
       }
     }
-  } else {
-    // Note: This currently assumes there are no passive effects on the root fiber
-    // because the root is not part of its own effect list.
-    // This could change in the future.
-    let effect = root.current.firstEffect;
-    while (effect !== null) {
+  }
+  // Note: This currently assumes there are no passive effects on the root fiber
+  // because the root is not part of its own effect list.
+  // This could change in the future.
+  let effect = root.current.firstEffect;
+  while (effect !== null) {
+    // We do this work above if this flag is enabled, so we shouldn't be
+    // doing it here.
+    if (!runAllPassiveEffectDestroysBeforeCreates) {
       if (__DEV__) {
         setCurrentDebugFiberInDEV(effect);
         invokeGuardedCallback(null, commitPassiveHookEffects, null, effect);
@@ -2391,12 +2394,12 @@ function flushPassiveEffectsImpl() {
           captureCommitPhaseError(effect, error);
         }
       }
-
-      const nextNextEffect = effect.nextEffect;
-      // Remove nextEffect pointer to assist GC
-      effect.nextEffect = null;
-      effect = nextNextEffect;
     }
+
+    const nextNextEffect = effect.nextEffect;
+    // Remove nextEffect pointer to assist GC
+    effect.nextEffect = null;
+    effect = nextNextEffect;
   }
 
   if (enableProfilerTimer && enableProfilerCommitHooks) {

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -2368,12 +2368,15 @@ function flushPassiveEffectsImpl() {
         }
       }
     }
-  } else {
-    // Note: This currently assumes there are no passive effects on the root fiber
-    // because the root is not part of its own effect list.
-    // This could change in the future.
-    let effect = root.current.firstEffect;
-    while (effect !== null) {
+  }
+  // Note: This currently assumes there are no passive effects on the root fiber
+  // because the root is not part of its own effect list.
+  // This could change in the future.
+  let effect = root.current.firstEffect;
+  while (effect !== null) {
+    // We do this work above if this flag is enabled, so we shouldn't be
+    // doing it here.
+    if (!runAllPassiveEffectDestroysBeforeCreates) {
       if (__DEV__) {
         setCurrentDebugFiberInDEV(effect);
         invokeGuardedCallback(null, commitPassiveHookEffects, null, effect);
@@ -2391,12 +2394,12 @@ function flushPassiveEffectsImpl() {
           captureCommitPhaseError(effect, error);
         }
       }
-
-      const nextNextEffect = effect.nextEffect;
-      // Remove nextEffect pointer to assist GC
-      effect.nextEffect = null;
-      effect = nextNextEffect;
     }
+
+    const nextNextEffect = effect.nextEffect;
+    // Remove nextEffect pointer to assist GC
+    effect.nextEffect = null;
+    effect = nextNextEffect;
   }
 
   if (enableProfilerTimer && enableProfilerCommitHooks) {