Skip to content

Commit d70ee32

Browse files
authored
[Flight] Eagerly parse stack traces in DebugNode (#33589)
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.
1 parent 6c7b1a1 commit d70ee32

File tree

3 files changed

+18
-19
lines changed

3 files changed

+18
-19
lines changed

packages/react-server/src/ReactFlightAsyncSequence.js

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@
77
* @flow
88
*/
99

10-
import type {ReactDebugInfo, ReactComponentInfo} from 'shared/ReactTypes';
10+
import type {
11+
ReactDebugInfo,
12+
ReactComponentInfo,
13+
ReactStackTrace,
14+
} from 'shared/ReactTypes';
1115

1216
export const IO_NODE = 0;
1317
export const PROMISE_NODE = 1;
@@ -22,7 +26,7 @@ type PromiseWithDebugInfo = interface extends Promise<any> {
2226
export type IONode = {
2327
tag: 0,
2428
owner: null | ReactComponentInfo,
25-
stack: Error, // callsite that spawned the I/O
29+
stack: ReactStackTrace, // callsite that spawned the I/O
2630
debugInfo: null, // not used on I/O
2731
start: number, // start time when the first part of the I/O sequence started
2832
end: number, // we typically don't use this. only when there's no promise intermediate.
@@ -34,7 +38,7 @@ export type PromiseNode = {
3438
tag: 1,
3539
owner: null | ReactComponentInfo,
3640
debugInfo: null | ReactDebugInfo, // forwarded debugInfo from the Promise
37-
stack: Error, // callsite that created the Promise
41+
stack: ReactStackTrace, // callsite that created the Promise
3842
start: number, // start time when the Promise was created
3943
end: number, // end time when the Promise was resolved.
4044
awaited: null | AsyncSequence, // the thing that ended up resolving this promise
@@ -45,7 +49,7 @@ export type AwaitNode = {
4549
tag: 2,
4650
owner: null | ReactComponentInfo,
4751
debugInfo: null | ReactDebugInfo, // forwarded debugInfo from the Promise
48-
stack: Error, // callsite that awaited (using await, .then(), Promise.all(), ...)
52+
stack: ReactStackTrace, // callsite that awaited (using await, .then(), Promise.all(), ...)
4953
start: number, // when we started blocking. This might be later than the I/O started.
5054
end: number, // when we unblocked. This might be later than the I/O resolved if there's CPU time.
5155
awaited: null | AsyncSequence, // the promise we were waiting on
@@ -56,7 +60,7 @@ export type UnresolvedPromiseNode = {
5660
tag: 3,
5761
owner: null | ReactComponentInfo,
5862
debugInfo: WeakRef<PromiseWithDebugInfo>, // holds onto the Promise until we can extract debugInfo when it resolves
59-
stack: Error, // callsite that created the Promise
63+
stack: ReactStackTrace, // callsite that created the Promise
6064
start: number, // start time when the Promise was created
6165
end: -1.1, // set when we resolve.
6266
awaited: null | AsyncSequence, // the thing that ended up resolving this promise
@@ -67,7 +71,7 @@ export type UnresolvedAwaitNode = {
6771
tag: 4,
6872
owner: null | ReactComponentInfo,
6973
debugInfo: WeakRef<PromiseWithDebugInfo>, // holds onto the Promise until we can extract debugInfo when it resolves
70-
stack: Error, // callsite that awaited (using await, .then(), Promise.all(), ...)
74+
stack: ReactStackTrace, // callsite that awaited (using await, .then(), Promise.all(), ...)
7175
start: number, // when we started blocking. This might be later than the I/O started.
7276
end: -1.1, // set when we resolve.
7377
awaited: null | AsyncSequence, // the promise we were waiting on

packages/react-server/src/ReactFlightServer.js

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1940,10 +1940,7 @@ function visitAsyncNode(
19401940
// If the ioNode was a Promise, then that means we found one in user space since otherwise
19411941
// we would've returned an IO node. We assume this has the best stack.
19421942
match = ioNode;
1943-
} else if (
1944-
filterStackTrace(request, parseStackTrace(node.stack, 1)).length ===
1945-
0
1946-
) {
1943+
} else if (filterStackTrace(request, node.stack).length === 0) {
19471944
// If this Promise was created inside only third party code, then try to use
19481945
// the inner I/O node instead. This could happen if third party calls into first
19491946
// party to perform some I/O.
@@ -1986,10 +1983,7 @@ function visitAsyncNode(
19861983
// just part of a previous component's rendering.
19871984
match = ioNode;
19881985
} else {
1989-
const stack = filterStackTrace(
1990-
request,
1991-
parseStackTrace(node.stack, 1),
1992-
);
1986+
const stack = filterStackTrace(request, node.stack);
19931987
if (stack.length === 0) {
19941988
// If this await was fully filtered out, then it was inside third party code
19951989
// such as in an external library. We return the I/O node and try another await.
@@ -3711,7 +3705,7 @@ function serializeIONode(
37113705
let stack = null;
37123706
let name = '';
37133707
if (ioNode.stack !== null) {
3714-
const fullStack = parseStackTrace(ioNode.stack, 1);
3708+
const fullStack = ioNode.stack;
37153709
stack = filterStackTrace(request, fullStack);
37163710
name = findCalledFunctionNameFromStackTrace(request, fullStack);
37173711
// The name can include the object that this was called on but sometimes that's

packages/react-server/src/ReactFlightServerConfigDebugNode.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
import {resolveOwner} from './flight/ReactFlightCurrentOwner';
2727
import {createHook, executionAsyncId, AsyncResource} from 'async_hooks';
2828
import {enableAsyncDebugInfo} from 'shared/ReactFeatureFlags';
29+
import {parseStackTrace} from './ReactFlightServerConfig';
2930

3031
// $FlowFixMe[method-unbinding]
3132
const getAsyncId = AsyncResource.prototype.asyncId;
@@ -86,7 +87,7 @@ export function initAsyncDebugInfo(): void {
8687
tag: UNRESOLVED_AWAIT_NODE,
8788
owner: resolveOwner(),
8889
debugInfo: new WeakRef((resource: Promise<any>)),
89-
stack: new Error(),
90+
stack: parseStackTrace(new Error(), 1),
9091
start: performance.now(),
9192
end: -1.1, // set when resolved.
9293
awaited: trigger, // The thing we're awaiting on. Might get overrriden when we resolve.
@@ -97,7 +98,7 @@ export function initAsyncDebugInfo(): void {
9798
tag: UNRESOLVED_PROMISE_NODE,
9899
owner: resolveOwner(),
99100
debugInfo: new WeakRef((resource: Promise<any>)),
100-
stack: new Error(),
101+
stack: parseStackTrace(new Error(), 1),
101102
start: performance.now(),
102103
end: -1.1, // Set when we resolve.
103104
awaited:
@@ -118,7 +119,7 @@ export function initAsyncDebugInfo(): void {
118119
tag: IO_NODE,
119120
owner: resolveOwner(),
120121
debugInfo: null,
121-
stack: new Error(), // This is only used if no native promises are used.
122+
stack: parseStackTrace(new Error(), 1), // This is only used if no native promises are used.
122123
start: performance.now(),
123124
end: -1.1, // Only set when pinged.
124125
awaited: null,
@@ -133,7 +134,7 @@ export function initAsyncDebugInfo(): void {
133134
tag: IO_NODE,
134135
owner: resolveOwner(),
135136
debugInfo: null,
136-
stack: new Error(),
137+
stack: parseStackTrace(new Error(), 1),
137138
start: performance.now(),
138139
end: -1.1, // Only set when pinged.
139140
awaited: null,

0 commit comments

Comments
 (0)