Repository: facebook/react
Pull request: [Flight] Eagerly parse stack traces in DebugNode

There's a memory leak in DebugNode where the `Error` objects that we instantiate retains their callstacks which can have Promises on them. In fact, it's very likely since the current callsite has the "resource" on it which is the Promise itself. If those Promises are retained then their `destroy` async hook is never fired which doesn't clean up our map which can contains the `Error` object. Creating a cycle that can't be cleaned up.

This fix is just eagerly reifying and parsing the stacks.

I totally expect this to be crazy slow since there's so many Promises that we end up not needing to visit otherwise. We'll need to optimize it somehow. Perhaps by being smarter about which ones we might need stacks for. However, at least it doesn't leak indefinitely.


Changed files:
filename: packages/react-server/src/ReactFlightAsyncSequence.js
patch: @@ -7,7 +7,11 @@
  * @flow
  */
 
-import type {ReactDebugInfo, ReactComponentInfo} from 'shared/ReactTypes';
+import type {
+  ReactDebugInfo,
+  ReactComponentInfo,
+  ReactStackTrace,
+} from 'shared/ReactTypes';
 
 export const IO_NODE = 0;
 export const PROMISE_NODE = 1;
@@ -22,7 +26,7 @@ type PromiseWithDebugInfo = interface extends Promise<any> {
 export type IONode = {
   tag: 0,
   owner: null | ReactComponentInfo,
-  stack: Error, // callsite that spawned the I/O
+  stack: ReactStackTrace, // callsite that spawned the I/O
   debugInfo: null, // not used on I/O
   start: number, // start time when the first part of the I/O sequence started
   end: number, // we typically don't use this. only when there's no promise intermediate.
@@ -34,7 +38,7 @@ export type PromiseNode = {
   tag: 1,
   owner: null | ReactComponentInfo,
   debugInfo: null | ReactDebugInfo, // forwarded debugInfo from the Promise
-  stack: Error, // callsite that created the Promise
+  stack: ReactStackTrace, // callsite that created the Promise
   start: number, // start time when the Promise was created
   end: number, // end time when the Promise was resolved.
   awaited: null | AsyncSequence, // the thing that ended up resolving this promise
@@ -45,7 +49,7 @@ export type AwaitNode = {
   tag: 2,
   owner: null | ReactComponentInfo,
   debugInfo: null | ReactDebugInfo, // forwarded debugInfo from the Promise
-  stack: Error, // callsite that awaited (using await, .then(), Promise.all(), ...)
+  stack: ReactStackTrace, // callsite that awaited (using await, .then(), Promise.all(), ...)
   start: number, // when we started blocking. This might be later than the I/O started.
   end: number, // when we unblocked. This might be later than the I/O resolved if there's CPU time.
   awaited: null | AsyncSequence, // the promise we were waiting on
@@ -56,7 +60,7 @@ export type UnresolvedPromiseNode = {
   tag: 3,
   owner: null | ReactComponentInfo,
   debugInfo: WeakRef<PromiseWithDebugInfo>, // holds onto the Promise until we can extract debugInfo when it resolves
-  stack: Error, // callsite that created the Promise
+  stack: ReactStackTrace, // callsite that created the Promise
   start: number, // start time when the Promise was created
   end: -1.1, // set when we resolve.
   awaited: null | AsyncSequence, // the thing that ended up resolving this promise
@@ -67,7 +71,7 @@ export type UnresolvedAwaitNode = {
   tag: 4,
   owner: null | ReactComponentInfo,
   debugInfo: WeakRef<PromiseWithDebugInfo>, // holds onto the Promise until we can extract debugInfo when it resolves
-  stack: Error, // callsite that awaited (using await, .then(), Promise.all(), ...)
+  stack: ReactStackTrace, // callsite that awaited (using await, .then(), Promise.all(), ...)
   start: number, // when we started blocking. This might be later than the I/O started.
   end: -1.1, // set when we resolve.
   awaited: null | AsyncSequence, // the promise we were waiting on

filename: packages/react-server/src/ReactFlightServer.js
patch: @@ -1940,10 +1940,7 @@ function visitAsyncNode(
             // If the ioNode was a Promise, then that means we found one in user space since otherwise
             // we would've returned an IO node. We assume this has the best stack.
             match = ioNode;
-          } else if (
-            filterStackTrace(request, parseStackTrace(node.stack, 1)).length ===
-            0
-          ) {
+          } else if (filterStackTrace(request, node.stack).length === 0) {
             // If this Promise was created inside only third party code, then try to use
             // the inner I/O node instead. This could happen if third party calls into first
             // party to perform some I/O.
@@ -1986,10 +1983,7 @@ function visitAsyncNode(
             // just part of a previous component's rendering.
             match = ioNode;
           } else {
-            const stack = filterStackTrace(
-              request,
-              parseStackTrace(node.stack, 1),
-            );
+            const stack = filterStackTrace(request, node.stack);
             if (stack.length === 0) {
               // If this await was fully filtered out, then it was inside third party code
               // such as in an external library. We return the I/O node and try another await.
@@ -3711,7 +3705,7 @@ function serializeIONode(
   let stack = null;
   let name = '';
   if (ioNode.stack !== null) {
-    const fullStack = parseStackTrace(ioNode.stack, 1);
+    const fullStack = ioNode.stack;
     stack = filterStackTrace(request, fullStack);
     name = findCalledFunctionNameFromStackTrace(request, fullStack);
     // The name can include the object that this was called on but sometimes that's

filename: packages/react-server/src/ReactFlightServerConfigDebugNode.js
patch: @@ -26,6 +26,7 @@ import {
 import {resolveOwner} from './flight/ReactFlightCurrentOwner';
 import {createHook, executionAsyncId, AsyncResource} from 'async_hooks';
 import {enableAsyncDebugInfo} from 'shared/ReactFeatureFlags';
+import {parseStackTrace} from './ReactFlightServerConfig';
 
 // $FlowFixMe[method-unbinding]
 const getAsyncId = AsyncResource.prototype.asyncId;
@@ -86,7 +87,7 @@ export function initAsyncDebugInfo(): void {
               tag: UNRESOLVED_AWAIT_NODE,
               owner: resolveOwner(),
               debugInfo: new WeakRef((resource: Promise<any>)),
-              stack: new Error(),
+              stack: parseStackTrace(new Error(), 1),
               start: performance.now(),
               end: -1.1, // set when resolved.
               awaited: trigger, // The thing we're awaiting on. Might get overrriden when we resolve.
@@ -97,7 +98,7 @@ export function initAsyncDebugInfo(): void {
               tag: UNRESOLVED_PROMISE_NODE,
               owner: resolveOwner(),
               debugInfo: new WeakRef((resource: Promise<any>)),
-              stack: new Error(),
+              stack: parseStackTrace(new Error(), 1),
               start: performance.now(),
               end: -1.1, // Set when we resolve.
               awaited:
@@ -118,7 +119,7 @@ export function initAsyncDebugInfo(): void {
               tag: IO_NODE,
               owner: resolveOwner(),
               debugInfo: null,
-              stack: new Error(), // This is only used if no native promises are used.
+              stack: parseStackTrace(new Error(), 1), // This is only used if no native promises are used.
               start: performance.now(),
               end: -1.1, // Only set when pinged.
               awaited: null,
@@ -133,7 +134,7 @@ export function initAsyncDebugInfo(): void {
               tag: IO_NODE,
               owner: resolveOwner(),
               debugInfo: null,
-              stack: new Error(),
+              stack: parseStackTrace(new Error(), 1),
               start: performance.now(),
               end: -1.1, // Only set when pinged.
               awaited: null,