Skip to content

Commit 5a9563a

Browse files
committed
fix(webapp): eliminate SSE abort-signal memory leak
AbortSignal.any() combined with abort(reason) on Node 20 pinned the full Express request/response graph via a DOMException stack trace and a FinalizationRegistry. Every aborted SSE connection leaked ~60KB (ServerResponse, IncomingMessage, Socket, 4x AbortController, SpanImpl) that survived GC indefinitely. - sse.ts: collapse the 4-signal stack to a single internalController.signal; replace AbortSignal.timeout() with a plain setTimeout cleared on abort; drop every string arg from .abort() so no DOMException stack is captured; removeEventListener the request-abort handler on cleanup. - entry.server.tsx: clear the setTimeout(abort, ABORT_DELAY) in every terminal callback so the abort closure does not pin the React render tree + remixContext for 30s per successful request (react-router #14200). - tracer.server.ts: gate HttpInstrumentation and ExpressInstrumentation behind DISABLE_HTTP_INSTRUMENTATION=true for isolation testing; defaults to enabled. Verified locally (500 aborted SSE connections + GC): - unpatched: +16.0 MB heap, 158 memlab leak clusters - patched: +3.3 MB heap, 0 app-code leaks
1 parent 2d3b2e8 commit 5a9563a

4 files changed

Lines changed: 52 additions & 41 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
Fix memory leak where every aborted SSE connection and every successful HTML render pinned the full request/response graph on Node 20, caused by `AbortSignal.any` + string abort reasons in `sse.ts` and an un-cleared `setTimeout(abort)` in `entry.server.tsx`.

apps/webapp/app/entry.server.tsx

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@ function handleBotRequest(
8383
) {
8484
return new Promise((resolve, reject) => {
8585
let shellRendered = false;
86+
// Timer handle is cleared in every terminal callback so the abort closure
87+
// (which captures the full React render tree + remixContext) doesn't pin
88+
// memory for 30s per successful request. See react-router PR #14200.
89+
let abortTimer: NodeJS.Timeout | undefined;
8690
const { pipe, abort } = renderToPipeableStream(
8791
<OperatingSystemContextProvider platform={platform}>
8892
<LocaleContextProvider locales={locales}>
@@ -105,8 +109,10 @@ function handleBotRequest(
105109
);
106110

107111
pipe(body);
112+
clearTimeout(abortTimer);
108113
},
109114
onShellError(error: unknown) {
115+
clearTimeout(abortTimer);
110116
reject(error);
111117
},
112118
onError(error: unknown) {
@@ -121,7 +127,7 @@ function handleBotRequest(
121127
}
122128
);
123129

124-
setTimeout(abort, ABORT_DELAY);
130+
abortTimer = setTimeout(abort, ABORT_DELAY);
125131
});
126132
}
127133

@@ -135,6 +141,10 @@ function handleBrowserRequest(
135141
) {
136142
return new Promise((resolve, reject) => {
137143
let shellRendered = false;
144+
// Timer handle is cleared in every terminal callback so the abort closure
145+
// (which captures the full React render tree + remixContext) doesn't pin
146+
// memory for 30s per successful request. See react-router PR #14200.
147+
let abortTimer: NodeJS.Timeout | undefined;
138148
const { pipe, abort } = renderToPipeableStream(
139149
<OperatingSystemContextProvider platform={platform}>
140150
<LocaleContextProvider locales={locales}>
@@ -157,8 +167,10 @@ function handleBrowserRequest(
157167
);
158168

159169
pipe(body);
170+
clearTimeout(abortTimer);
160171
},
161172
onShellError(error: unknown) {
173+
clearTimeout(abortTimer);
162174
reject(error);
163175
},
164176
onError(error: unknown) {
@@ -173,7 +185,7 @@ function handleBrowserRequest(
173185
}
174186
);
175187

176-
setTimeout(abort, ABORT_DELAY);
188+
abortTimer = setTimeout(abort, ABORT_DELAY);
177189
});
178190
}
179191

apps/webapp/app/utils/sse.ts

Lines changed: 28 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ export function createSSELoader(options: SSEOptions) {
4545
const id = request.headers.get("x-request-id") || Math.random().toString(36).slice(2, 8);
4646

4747
const internalController = new AbortController();
48-
const timeoutSignal = AbortSignal.timeout(timeout);
4948

5049
const log = (message: string) => {
5150
if (debug)
@@ -60,16 +59,14 @@ export function createSSELoader(options: SSEOptions) {
6059
if (!internalController.signal.aborted) {
6160
originalSend(event);
6261
}
63-
// If controller is aborted, silently ignore the send attempt
6462
} catch (error) {
6563
if (error instanceof Error) {
6664
if (error.message?.includes("Controller is already closed")) {
67-
// Silently handle controller closed errors
6865
return;
6966
}
7067
log(`Error sending event: ${error.message}`);
7168
}
72-
throw error; // Re-throw other errors
69+
throw error;
7370
}
7471
};
7572
};
@@ -92,51 +89,42 @@ export function createSSELoader(options: SSEOptions) {
9289

9390
const requestAbortSignal = getRequestAbortSignal();
9491

95-
const combinedSignal = AbortSignal.any([
96-
requestAbortSignal,
97-
timeoutSignal,
98-
internalController.signal,
99-
]);
100-
10192
log("Start");
10293

103-
requestAbortSignal.addEventListener(
104-
"abort",
105-
() => {
106-
log(`request signal aborted`);
107-
internalController.abort("Request aborted");
108-
},
109-
{ once: true, signal: internalController.signal }
110-
);
111-
112-
combinedSignal.addEventListener(
113-
"abort",
114-
() => {
115-
log(`combinedSignal aborted: ${combinedSignal.reason}`);
116-
},
117-
{ once: true, signal: internalController.signal }
118-
);
94+
// Single-signal abort chain: everything rolls up into internalController with NO
95+
// string reasons (string reasons create a DOMException whose stack trace pins the
96+
// closure graph). Timeout is a plain setTimeout cleared on abort rather than an
97+
// AbortSignal.timeout() combined via AbortSignal.any(); both of those patterns
98+
// leak on Node 20 due to FinalizationRegistry tracking of dependent signals.
99+
const timeoutTimer = setTimeout(() => {
100+
if (!internalController.signal.aborted) internalController.abort();
101+
}, timeout);
102+
103+
const onRequestAbort = () => {
104+
log("request signal aborted");
105+
if (!internalController.signal.aborted) internalController.abort();
106+
};
107+
requestAbortSignal.addEventListener("abort", onRequestAbort, { once: true });
119108

120-
timeoutSignal.addEventListener(
109+
internalController.signal.addEventListener(
121110
"abort",
122111
() => {
123-
if (internalController.signal.aborted) return;
124-
log(`timeoutSignal aborted: ${timeoutSignal.reason}`);
125-
internalController.abort("Timeout");
112+
clearTimeout(timeoutTimer);
113+
requestAbortSignal.removeEventListener("abort", onRequestAbort);
126114
},
127-
{ once: true, signal: internalController.signal }
115+
{ once: true }
128116
);
129117

130118
if (handlers.beforeStream) {
131119
const shouldContinue = await handlers.beforeStream();
132120
if (shouldContinue === false) {
133121
log("beforeStream returned false, so we'll exit before creating the stream");
134-
internalController.abort("Init requested stop");
122+
internalController.abort();
135123
return;
136124
}
137125
}
138126

139-
return eventStream(combinedSignal, function setup(send) {
127+
return eventStream(internalController.signal, function setup(send) {
140128
connections.add(id);
141129
const safeSend = createSafeSend(send);
142130

@@ -147,14 +135,14 @@ export function createSSELoader(options: SSEOptions) {
147135
const shouldContinue = await handlers.initStream({ send: safeSend });
148136
if (shouldContinue === false) {
149137
log("initStream returned false, so we'll stop the stream");
150-
internalController.abort("Init requested stop");
138+
internalController.abort();
151139
return;
152140
}
153141
}
154142

155143
log("Starting interval");
156144
for await (const _ of setInterval(interval, null, {
157-
signal: combinedSignal,
145+
signal: internalController.signal,
158146
})) {
159147
log("PING");
160148

@@ -165,13 +153,16 @@ export function createSSELoader(options: SSEOptions) {
165153
const shouldContinue = await handlers.iterator({ date, send: safeSend });
166154
if (shouldContinue === false) {
167155
log("iterator return false, so we'll stop the stream");
168-
internalController.abort("Iterator requested stop");
156+
internalController.abort();
169157
break;
170158
}
171159
} catch (error) {
172160
log("iterator threw an error, aborting stream");
173161
// Immediately abort to trigger cleanup
174-
internalController.abort(error instanceof Error ? error.message : "Iterator error");
162+
if (error instanceof Error && error.name !== "AbortError") {
163+
log(`iterator error: ${error.message}`);
164+
}
165+
internalController.abort();
175166
// No need to re-throw as we're handling it by aborting
176167
return; // Exit the run function immediately
177168
}

apps/webapp/app/v3/tracer.server.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,13 +302,15 @@ function setupTelemetry() {
302302
provider.register();
303303

304304
let instrumentations: Instrumentation[] = [
305-
new HttpInstrumentation(),
306-
new ExpressInstrumentation(),
307305
new AwsSdkInstrumentation({
308306
suppressInternalInstrumentation: true,
309307
}),
310308
];
311309

310+
if (process.env.DISABLE_HTTP_INSTRUMENTATION !== "true") {
311+
instrumentations.unshift(new HttpInstrumentation(), new ExpressInstrumentation());
312+
}
313+
312314
if (env.INTERNAL_OTEL_TRACE_INSTRUMENT_PRISMA_ENABLED === "1") {
313315
instrumentations.push(new PrismaInstrumentation());
314316
}

0 commit comments

Comments
 (0)