From 5a9563a9afaaf0dfe3d4de881cef65fc5a7d76c7 Mon Sep 17 00:00:00 2001 From: Eric Allam Date: Thu, 23 Apr 2026 11:44:05 +0100 Subject: [PATCH 1/5] 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 --- .server-changes/fix-sse-memory-leak.md | 6 +++ apps/webapp/app/entry.server.tsx | 16 ++++++- apps/webapp/app/utils/sse.ts | 65 +++++++++++--------------- apps/webapp/app/v3/tracer.server.ts | 6 ++- 4 files changed, 52 insertions(+), 41 deletions(-) create mode 100644 .server-changes/fix-sse-memory-leak.md diff --git a/.server-changes/fix-sse-memory-leak.md b/.server-changes/fix-sse-memory-leak.md new file mode 100644 index 00000000000..9f341594595 --- /dev/null +++ b/.server-changes/fix-sse-memory-leak.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: fix +--- + +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`. diff --git a/apps/webapp/app/entry.server.tsx b/apps/webapp/app/entry.server.tsx index 4ee4f252a32..87171011e03 100644 --- a/apps/webapp/app/entry.server.tsx +++ b/apps/webapp/app/entry.server.tsx @@ -83,6 +83,10 @@ function handleBotRequest( ) { return new Promise((resolve, reject) => { let shellRendered = false; + // Timer handle is cleared in every terminal callback so the abort closure + // (which captures the full React render tree + remixContext) doesn't pin + // memory for 30s per successful request. See react-router PR #14200. + let abortTimer: NodeJS.Timeout | undefined; const { pipe, abort } = renderToPipeableStream( @@ -105,8 +109,10 @@ function handleBotRequest( ); pipe(body); + clearTimeout(abortTimer); }, onShellError(error: unknown) { + clearTimeout(abortTimer); reject(error); }, onError(error: unknown) { @@ -121,7 +127,7 @@ function handleBotRequest( } ); - setTimeout(abort, ABORT_DELAY); + abortTimer = setTimeout(abort, ABORT_DELAY); }); } @@ -135,6 +141,10 @@ function handleBrowserRequest( ) { return new Promise((resolve, reject) => { let shellRendered = false; + // Timer handle is cleared in every terminal callback so the abort closure + // (which captures the full React render tree + remixContext) doesn't pin + // memory for 30s per successful request. See react-router PR #14200. + let abortTimer: NodeJS.Timeout | undefined; const { pipe, abort } = renderToPipeableStream( @@ -157,8 +167,10 @@ function handleBrowserRequest( ); pipe(body); + clearTimeout(abortTimer); }, onShellError(error: unknown) { + clearTimeout(abortTimer); reject(error); }, onError(error: unknown) { @@ -173,7 +185,7 @@ function handleBrowserRequest( } ); - setTimeout(abort, ABORT_DELAY); + abortTimer = setTimeout(abort, ABORT_DELAY); }); } diff --git a/apps/webapp/app/utils/sse.ts b/apps/webapp/app/utils/sse.ts index f48cc9e31f9..5a0ced3e08b 100644 --- a/apps/webapp/app/utils/sse.ts +++ b/apps/webapp/app/utils/sse.ts @@ -45,7 +45,6 @@ export function createSSELoader(options: SSEOptions) { const id = request.headers.get("x-request-id") || Math.random().toString(36).slice(2, 8); const internalController = new AbortController(); - const timeoutSignal = AbortSignal.timeout(timeout); const log = (message: string) => { if (debug) @@ -60,16 +59,14 @@ export function createSSELoader(options: SSEOptions) { if (!internalController.signal.aborted) { originalSend(event); } - // If controller is aborted, silently ignore the send attempt } catch (error) { if (error instanceof Error) { if (error.message?.includes("Controller is already closed")) { - // Silently handle controller closed errors return; } log(`Error sending event: ${error.message}`); } - throw error; // Re-throw other errors + throw error; } }; }; @@ -92,51 +89,42 @@ export function createSSELoader(options: SSEOptions) { const requestAbortSignal = getRequestAbortSignal(); - const combinedSignal = AbortSignal.any([ - requestAbortSignal, - timeoutSignal, - internalController.signal, - ]); - log("Start"); - requestAbortSignal.addEventListener( - "abort", - () => { - log(`request signal aborted`); - internalController.abort("Request aborted"); - }, - { once: true, signal: internalController.signal } - ); - - combinedSignal.addEventListener( - "abort", - () => { - log(`combinedSignal aborted: ${combinedSignal.reason}`); - }, - { once: true, signal: internalController.signal } - ); + // Single-signal abort chain: everything rolls up into internalController with NO + // string reasons (string reasons create a DOMException whose stack trace pins the + // closure graph). Timeout is a plain setTimeout cleared on abort rather than an + // AbortSignal.timeout() combined via AbortSignal.any(); both of those patterns + // leak on Node 20 due to FinalizationRegistry tracking of dependent signals. + const timeoutTimer = setTimeout(() => { + if (!internalController.signal.aborted) internalController.abort(); + }, timeout); + + const onRequestAbort = () => { + log("request signal aborted"); + if (!internalController.signal.aborted) internalController.abort(); + }; + requestAbortSignal.addEventListener("abort", onRequestAbort, { once: true }); - timeoutSignal.addEventListener( + internalController.signal.addEventListener( "abort", () => { - if (internalController.signal.aborted) return; - log(`timeoutSignal aborted: ${timeoutSignal.reason}`); - internalController.abort("Timeout"); + clearTimeout(timeoutTimer); + requestAbortSignal.removeEventListener("abort", onRequestAbort); }, - { once: true, signal: internalController.signal } + { once: true } ); if (handlers.beforeStream) { const shouldContinue = await handlers.beforeStream(); if (shouldContinue === false) { log("beforeStream returned false, so we'll exit before creating the stream"); - internalController.abort("Init requested stop"); + internalController.abort(); return; } } - return eventStream(combinedSignal, function setup(send) { + return eventStream(internalController.signal, function setup(send) { connections.add(id); const safeSend = createSafeSend(send); @@ -147,14 +135,14 @@ export function createSSELoader(options: SSEOptions) { const shouldContinue = await handlers.initStream({ send: safeSend }); if (shouldContinue === false) { log("initStream returned false, so we'll stop the stream"); - internalController.abort("Init requested stop"); + internalController.abort(); return; } } log("Starting interval"); for await (const _ of setInterval(interval, null, { - signal: combinedSignal, + signal: internalController.signal, })) { log("PING"); @@ -165,13 +153,16 @@ export function createSSELoader(options: SSEOptions) { const shouldContinue = await handlers.iterator({ date, send: safeSend }); if (shouldContinue === false) { log("iterator return false, so we'll stop the stream"); - internalController.abort("Iterator requested stop"); + internalController.abort(); break; } } catch (error) { log("iterator threw an error, aborting stream"); // Immediately abort to trigger cleanup - internalController.abort(error instanceof Error ? error.message : "Iterator error"); + if (error instanceof Error && error.name !== "AbortError") { + log(`iterator error: ${error.message}`); + } + internalController.abort(); // No need to re-throw as we're handling it by aborting return; // Exit the run function immediately } diff --git a/apps/webapp/app/v3/tracer.server.ts b/apps/webapp/app/v3/tracer.server.ts index 2ce5aa275c7..1750eb9f7d1 100644 --- a/apps/webapp/app/v3/tracer.server.ts +++ b/apps/webapp/app/v3/tracer.server.ts @@ -302,13 +302,15 @@ function setupTelemetry() { provider.register(); let instrumentations: Instrumentation[] = [ - new HttpInstrumentation(), - new ExpressInstrumentation(), new AwsSdkInstrumentation({ suppressInternalInstrumentation: true, }), ]; + if (process.env.DISABLE_HTTP_INSTRUMENTATION !== "true") { + instrumentations.unshift(new HttpInstrumentation(), new ExpressInstrumentation()); + } + if (env.INTERNAL_OTEL_TRACE_INSTRUMENT_PRISMA_ENABLED === "1") { instrumentations.push(new PrismaInstrumentation()); } From 858e1d697675608019b78eb8c4867b1985da518f Mon Sep 17 00:00:00 2001 From: Eric Allam Date: Thu, 23 Apr 2026 12:44:43 +0100 Subject: [PATCH 2/5] fixes for pr review feedback --- apps/webapp/app/env.server.ts | 1 + apps/webapp/app/utils/sse.ts | 16 +++++++++++++++- apps/webapp/app/v3/tracer.server.ts | 2 +- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/apps/webapp/app/env.server.ts b/apps/webapp/app/env.server.ts index ba40624058f..06300e13607 100644 --- a/apps/webapp/app/env.server.ts +++ b/apps/webapp/app/env.server.ts @@ -438,6 +438,7 @@ const EnvironmentSchema = z INTERNAL_OTEL_TRACE_SAMPLING_RATE: z.string().default("20"), INTERNAL_OTEL_TRACE_INSTRUMENT_PRISMA_ENABLED: z.string().default("0"), INTERNAL_OTEL_TRACE_DISABLED: z.string().default("0"), + DISABLE_HTTP_INSTRUMENTATION: z.string().default("false"), INTERNAL_OTEL_LOG_EXPORTER_URL: z.string().optional(), INTERNAL_OTEL_METRIC_EXPORTER_URL: z.string().optional(), diff --git a/apps/webapp/app/utils/sse.ts b/apps/webapp/app/utils/sse.ts index 5a0ced3e08b..75e9dadbb9d 100644 --- a/apps/webapp/app/utils/sse.ts +++ b/apps/webapp/app/utils/sse.ts @@ -66,6 +66,12 @@ export function createSSELoader(options: SSEOptions) { } log(`Error sending event: ${error.message}`); } + // Abort before rethrowing so timer + request-abort listener are cleaned + // up immediately. Otherwise a send-failure in initStream leaves them + // alive until `timeout` fires. + if (!internalController.signal.aborted) { + internalController.abort(); + } throw error; } }; @@ -104,7 +110,6 @@ export function createSSELoader(options: SSEOptions) { log("request signal aborted"); if (!internalController.signal.aborted) internalController.abort(); }; - requestAbortSignal.addEventListener("abort", onRequestAbort, { once: true }); internalController.signal.addEventListener( "abort", @@ -115,6 +120,15 @@ export function createSSELoader(options: SSEOptions) { { once: true } ); + // The request could have been aborted during `await handler(context)` above. + // AbortSignal listeners added after the signal is already aborted never fire, + // so invoke cleanup synchronously in that case instead of waiting for `timeout`. + if (requestAbortSignal.aborted) { + onRequestAbort(); + } else { + requestAbortSignal.addEventListener("abort", onRequestAbort, { once: true }); + } + if (handlers.beforeStream) { const shouldContinue = await handlers.beforeStream(); if (shouldContinue === false) { diff --git a/apps/webapp/app/v3/tracer.server.ts b/apps/webapp/app/v3/tracer.server.ts index 1750eb9f7d1..e2bac269ffb 100644 --- a/apps/webapp/app/v3/tracer.server.ts +++ b/apps/webapp/app/v3/tracer.server.ts @@ -307,7 +307,7 @@ function setupTelemetry() { }), ]; - if (process.env.DISABLE_HTTP_INSTRUMENTATION !== "true") { + if (env.DISABLE_HTTP_INSTRUMENTATION !== "true") { instrumentations.unshift(new HttpInstrumentation(), new ExpressInstrumentation()); } From 3cebc05453c80676f7a2afe75ba376a09a763280 Mon Sep 17 00:00:00 2001 From: Eric Allam Date: Thu, 23 Apr 2026 13:35:26 +0100 Subject: [PATCH 3/5] fix(webapp): use BoolEnv for DISABLE_HTTP_INSTRUMENTATION Switch DISABLE_HTTP_INSTRUMENTATION from z.string().default("false") to BoolEnv.default(false) for consistency with other boolean env flags, and update the tracer check to use the parsed boolean directly. --- apps/webapp/app/env.server.ts | 2 +- apps/webapp/app/v3/tracer.server.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/webapp/app/env.server.ts b/apps/webapp/app/env.server.ts index 06300e13607..c10446d08ab 100644 --- a/apps/webapp/app/env.server.ts +++ b/apps/webapp/app/env.server.ts @@ -438,7 +438,7 @@ const EnvironmentSchema = z INTERNAL_OTEL_TRACE_SAMPLING_RATE: z.string().default("20"), INTERNAL_OTEL_TRACE_INSTRUMENT_PRISMA_ENABLED: z.string().default("0"), INTERNAL_OTEL_TRACE_DISABLED: z.string().default("0"), - DISABLE_HTTP_INSTRUMENTATION: z.string().default("false"), + DISABLE_HTTP_INSTRUMENTATION: BoolEnv.default(false), INTERNAL_OTEL_LOG_EXPORTER_URL: z.string().optional(), INTERNAL_OTEL_METRIC_EXPORTER_URL: z.string().optional(), diff --git a/apps/webapp/app/v3/tracer.server.ts b/apps/webapp/app/v3/tracer.server.ts index e2bac269ffb..1115ab42de8 100644 --- a/apps/webapp/app/v3/tracer.server.ts +++ b/apps/webapp/app/v3/tracer.server.ts @@ -307,7 +307,7 @@ function setupTelemetry() { }), ]; - if (env.DISABLE_HTTP_INSTRUMENTATION !== "true") { + if (!env.DISABLE_HTTP_INSTRUMENTATION) { instrumentations.unshift(new HttpInstrumentation(), new ExpressInstrumentation()); } From c2e6a2d3e028a51d8970832e555351bef16f88a4 Mon Sep 17 00:00:00 2001 From: Eric Allam Date: Thu, 23 Apr 2026 13:49:18 +0100 Subject: [PATCH 4/5] fix(webapp): drop string arg from RunStreamPresenter abort The same internalController is exposed to handlers via context.controller. RunStreamPresenter was still calling .abort("Send error"), which creates a DOMException with a stack trace on signal.reason. Without AbortSignal.any this no longer pins the closure graph, but it's inconsistent with the rest of the fix and worth cleaning up. --- apps/webapp/app/presenters/v3/RunStreamPresenter.server.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/webapp/app/presenters/v3/RunStreamPresenter.server.ts b/apps/webapp/app/presenters/v3/RunStreamPresenter.server.ts index 1dd4edc6233..393769bb1fe 100644 --- a/apps/webapp/app/presenters/v3/RunStreamPresenter.server.ts +++ b/apps/webapp/app/presenters/v3/RunStreamPresenter.server.ts @@ -66,8 +66,10 @@ export class RunStreamPresenter { }); } } - // Abort the stream on send error - context.controller.abort("Send error"); + // Abort the stream on send error. No reason string — string reasons + // create a DOMException whose stack trace captures the surrounding + // closure (see sse.ts comment). + context.controller.abort(); } }, 1000 From 6ae1a1ffa42f2ddac24ed1468136243ec21e6786 Mon Sep 17 00:00:00 2001 From: Eric Allam Date: Thu, 23 Apr 2026 14:24:14 +0100 Subject: [PATCH 5/5] fix(webapp): named sentinel abort reasons + cite Node issues After verifying the mechanism with a standalone isolation test (see PR), AbortSignal.any's dependent-signal tracking is the sole cause of the leak; the reason type (.abort() vs .abort("string")) is not. Keeping the signal chain collapsed to a single AbortController is what eliminates retention. - Add named sentinel constants (ABORT_REASON_*) for readability and to satisfy the CLAUDE.md named-constant rule. - Route RunStreamPresenter's send-error abort through the shared ABORT_REASON_SEND_ERROR sentinel. - Update the sse.ts comment to cite nodejs/node#54614 (production shape reported by ChainSafe Lodestar in the same retention pattern) and nodejs/node#55351 (mechanism confirmed by @jasnell, narrow fix shipped in 22.12.0 via #55354). - Correct the .server-changes entry: the leak is caused by AbortSignal.any, not by the string abort reasons (the earlier phrasing conflated the two). --- .server-changes/fix-sse-memory-leak.md | 2 +- .../v3/RunStreamPresenter.server.ts | 10 ++--- apps/webapp/app/utils/sse.ts | 45 ++++++++++++++----- 3 files changed, 39 insertions(+), 18 deletions(-) diff --git a/.server-changes/fix-sse-memory-leak.md b/.server-changes/fix-sse-memory-leak.md index 9f341594595..e2b9ddd1810 100644 --- a/.server-changes/fix-sse-memory-leak.md +++ b/.server-changes/fix-sse-memory-leak.md @@ -3,4 +3,4 @@ area: webapp type: fix --- -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`. +Fix memory leak where every aborted SSE connection pinned the full request/response graph on Node 20, caused by `AbortSignal.any()` in `sse.ts` retaining its source signals indefinitely (see nodejs/node#54614, nodejs/node#55351). Also clear the `setTimeout(abort)` timer in `entry.server.tsx` so successful HTML renders don't pin the React tree for 30s per request. diff --git a/apps/webapp/app/presenters/v3/RunStreamPresenter.server.ts b/apps/webapp/app/presenters/v3/RunStreamPresenter.server.ts index 393769bb1fe..69560c49e88 100644 --- a/apps/webapp/app/presenters/v3/RunStreamPresenter.server.ts +++ b/apps/webapp/app/presenters/v3/RunStreamPresenter.server.ts @@ -1,7 +1,7 @@ import { type PrismaClient, prisma } from "~/db.server"; import { logger } from "~/services/logger.server"; import { singleton } from "~/utils/singleton"; -import { createSSELoader, SendFunction } from "~/utils/sse"; +import { ABORT_REASON_SEND_ERROR, createSSELoader, SendFunction } from "~/utils/sse"; import { throttle } from "~/utils/throttle"; import { tracePubSub } from "~/v3/services/tracePubSub.server"; @@ -66,10 +66,10 @@ export class RunStreamPresenter { }); } } - // Abort the stream on send error. No reason string — string reasons - // create a DOMException whose stack trace captures the surrounding - // closure (see sse.ts comment). - context.controller.abort(); + // Abort the stream on send error. Uses a stackless string sentinel + // from sse.ts — a no-arg abort() would create a DOMException with a + // stack trace, which is unnecessary retention on the signal.reason. + context.controller.abort(ABORT_REASON_SEND_ERROR); } }, 1000 diff --git a/apps/webapp/app/utils/sse.ts b/apps/webapp/app/utils/sse.ts index 75e9dadbb9d..53f9aa010cd 100644 --- a/apps/webapp/app/utils/sse.ts +++ b/apps/webapp/app/utils/sse.ts @@ -38,6 +38,20 @@ type SSEOptions = { // This is used to track the open connections, for debugging const connections: Set = new Set(); +// Stackless sentinel reasons passed to AbortController#abort. Calling .abort() +// with no argument produces a DOMException that captures a ~500-byte stack +// trace; a string reason is stored verbatim with no stack. The choice of +// reason type does not cause the retention we saw in prod (that was the +// AbortSignal.any composite — see comment near the timeoutTimer below for the +// Node issue refs), but naming the sentinels keeps call sites readable and +// lets future signal.reason consumers branch on the cause. +export const ABORT_REASON_REQUEST = "request_aborted"; +export const ABORT_REASON_TIMEOUT = "timeout"; +export const ABORT_REASON_SEND_ERROR = "send_error"; +export const ABORT_REASON_INIT_STOP = "init_requested_stop"; +export const ABORT_REASON_ITERATOR_STOP = "iterator_requested_stop"; +export const ABORT_REASON_ITERATOR_ERROR = "iterator_error"; + export function createSSELoader(options: SSEOptions) { const { timeout, interval = 500, debug = false, handler } = options; @@ -70,7 +84,7 @@ export function createSSELoader(options: SSEOptions) { // up immediately. Otherwise a send-failure in initStream leaves them // alive until `timeout` fires. if (!internalController.signal.aborted) { - internalController.abort(); + internalController.abort(ABORT_REASON_SEND_ERROR); } throw error; } @@ -97,18 +111,25 @@ export function createSSELoader(options: SSEOptions) { log("Start"); - // Single-signal abort chain: everything rolls up into internalController with NO - // string reasons (string reasons create a DOMException whose stack trace pins the - // closure graph). Timeout is a plain setTimeout cleared on abort rather than an - // AbortSignal.timeout() combined via AbortSignal.any(); both of those patterns - // leak on Node 20 due to FinalizationRegistry tracking of dependent signals. + // Single-signal abort chain: everything rolls up into internalController. + // Timeout is a plain setTimeout cleared on abort rather than an + // AbortSignal.timeout() combined via AbortSignal.any() — AbortSignal.any + // keeps its source signals in an internal Set managed by a + // FinalizationRegistry, and under sustained request traffic those entries + // accumulate faster than they get cleaned up, pinning every source signal + // (and its listeners, and anything those listeners close over) until the + // parent signal is GC'd or aborts. Reproduced locally in isolation; shape + // matches the ChainSafe Lodestar production case described in + // nodejs/node#54614. See also nodejs/node#55351 (mechanism confirmed by + // @jasnell, narrow fix in 22.12.0 via #55354) and nodejs/node#57584 + // (circular-dep variant, still open). const timeoutTimer = setTimeout(() => { - if (!internalController.signal.aborted) internalController.abort(); + if (!internalController.signal.aborted) internalController.abort(ABORT_REASON_TIMEOUT); }, timeout); const onRequestAbort = () => { log("request signal aborted"); - if (!internalController.signal.aborted) internalController.abort(); + if (!internalController.signal.aborted) internalController.abort(ABORT_REASON_REQUEST); }; internalController.signal.addEventListener( @@ -133,7 +154,7 @@ export function createSSELoader(options: SSEOptions) { const shouldContinue = await handlers.beforeStream(); if (shouldContinue === false) { log("beforeStream returned false, so we'll exit before creating the stream"); - internalController.abort(); + internalController.abort(ABORT_REASON_INIT_STOP); return; } } @@ -149,7 +170,7 @@ export function createSSELoader(options: SSEOptions) { const shouldContinue = await handlers.initStream({ send: safeSend }); if (shouldContinue === false) { log("initStream returned false, so we'll stop the stream"); - internalController.abort(); + internalController.abort(ABORT_REASON_INIT_STOP); return; } } @@ -167,7 +188,7 @@ export function createSSELoader(options: SSEOptions) { const shouldContinue = await handlers.iterator({ date, send: safeSend }); if (shouldContinue === false) { log("iterator return false, so we'll stop the stream"); - internalController.abort(); + internalController.abort(ABORT_REASON_ITERATOR_STOP); break; } } catch (error) { @@ -176,7 +197,7 @@ export function createSSELoader(options: SSEOptions) { if (error instanceof Error && error.name !== "AbortError") { log(`iterator error: ${error.message}`); } - internalController.abort(); + internalController.abort(ABORT_REASON_ITERATOR_ERROR); // No need to re-throw as we're handling it by aborting return; // Exit the run function immediately }