diff --git a/packages/react-client/src/ReactFlightReplyClient.js b/packages/react-client/src/ReactFlightReplyClient.js index 0a4b0edce8e..20282c441e4 100644 --- a/packages/react-client/src/ReactFlightReplyClient.js +++ b/packages/react-client/src/ReactFlightReplyClient.js @@ -39,6 +39,12 @@ import getPrototypeOf from 'shared/getPrototypeOf'; const ObjectPrototype = Object.prototype; +// Passed to replyLifetimeController.abort(). Nothing reads the reason, but a +// call to abort() without one constructs an AbortError DOMException. Capturing +// the stack trace dominates that cost, and the cost grows with the depth of the +// stack. +const REPLY_ENDED = 'The reply ended.'; + import { usedWithSSR, checkEvalAvailabilityOnceDev, @@ -184,14 +190,58 @@ export function processReply( root: ReactServerValue, formFieldPrefix: string, temporaryReferences: void | TemporaryReferenceSet, - resolve: (string | FormData) => void, - reject: (error: mixed) => void, -): (reason: mixed) => void { + onResolve: (string | FormData) => void, + onReject: (error: mixed) => void, + signal: void | AbortSignal, +): void { let nextPartId = 1; let pendingParts = 0; let formData: null | FormData = null; const writtenObjects: WeakMap = new WeakMap(); let modelRoot: null | ReactServerValue = root; + let settled = false; + // Bounds the abort listener that attachAbortSignal attaches to the caller's + // signal. Null until a signal is attached, so a reply that gets no signal + // never creates a controller. + let replyLifetimeController: null | AbortController = null; + + // Ending the lifetime makes the runtime remove the caller's abort listener. + // Without that, the listener keeps everything this reply serialized reachable + // for as long as the caller's signal lives, and a composite signal from + // AbortSignal.any() is itself retained by the runtime while it has any abort + // listener attached. + function endReplyLifetime(): void { + if (replyLifetimeController !== null) { + replyLifetimeController.abort(REPLY_ENDED); + } + } + + function resolve(value: string | FormData): void { + settled = true; + endReplyLifetime(); + onResolve(value); + } + + function reject(error: mixed): void { + settled = true; + endReplyLifetime(); + onReject(error); + } + + function attachAbortSignal(abortSignal: AbortSignal): void { + if (abortSignal.aborted) { + abort(abortSignal.reason); + return; + } + replyLifetimeController = new AbortController(); + abortSignal.addEventListener( + 'abort', + () => { + abort(abortSignal.reason); + }, + {signal: replyLifetimeController.signal}, + ); + } if (__DEV__) { // We use eval to create fake function stacks which includes Component stacks. @@ -894,6 +944,9 @@ export function processReply( } function abort(reason: mixed): void { + // Nothing can make the reply pending again from here, so the caller's + // signal has no further effect on it. + endReplyLifetime(); if (pendingParts > 0) { pendingParts = 0; // Don't resolve again later. // Resolve with what we have so far, which may have holes at this point. @@ -920,7 +973,17 @@ export function processReply( } } - return abort; + // Wired up after serializing: abort() reads `json` and resolves with the + // parts that finished, so it must not be reachable before then. A reply that + // already settled gets no listener, since aborting it would be a no-op and + // the lifetime that removes the listener has already ended. + // + // TODO: Skip serializing when the signal is already aborted, the way the + // server entry points abort before rendering starts. Needs a decision on what + // to resolve with, since abort() resolves with the parts that finished. + if (signal !== undefined && !settled) { + attachAbortSignal(signal); + } } const boundCache: WeakMap< diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js index ea52339a869..6be61ffc2ca 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js @@ -1441,4 +1441,162 @@ describe('ReactDOMFizzStaticBrowser', () => { expect(getVisibleChildren(container)).toEqual(
Hi
); }); + + describe('abort signal lifetime', () => { + // Collects the lifetime signal that React bounds each abort listener with. + // React passes that signal to addEventListener instead of calling + // removeEventListener, so the runtime performs the removal and nothing here + // observes it directly. An aborted lifetime is what shows the listener is + // gone. + function trackAbortListenerLifetimes(signal) { + const lifetimes = []; + const add = signal.addEventListener.bind(signal); + signal.addEventListener = (type, listener, options) => { + if (type === 'abort') { + lifetimes.push(options.signal); + } + return add(type, listener, options); + }; + return lifetimes; + } + + it('detaches the listener when a prerender completes', async () => { + const controller = new AbortController(); + const lifetimes = trackAbortListenerLifetimes(controller.signal); + + const result = await serverAct(() => + ReactDOMFizzStatic.prerender(
hello world
, { + signal: controller.signal, + }), + ); + expect(lifetimes).toHaveLength(1); + expect(lifetimes[0].aborted).toBe(false); + + await readContent(result.prelude); + expect(lifetimes[0].aborted).toBe(true); + }); + + it('detaches the listener when a render completes', async () => { + const controller = new AbortController(); + const lifetimes = trackAbortListenerLifetimes(controller.signal); + + const stream = await serverAct(() => + ReactDOMFizzServer.renderToReadableStream(
hello world
, { + signal: controller.signal, + }), + ); + expect(lifetimes).toHaveLength(1); + expect(lifetimes[0].aborted).toBe(false); + + await readContent(stream); + expect(lifetimes[0].aborted).toBe(true); + }); + + it('detaches the listener when the signal aborts mid-render', async () => { + let hasLoaded = false; + let resolve; + const promise = new Promise(r => (resolve = r)); + function Wait() { + if (!hasLoaded) { + throw promise; + } + return 'Done'; + } + + const controller = new AbortController(); + const lifetimes = trackAbortListenerLifetimes(controller.signal); + + const resultPromise = ReactDOMFizzStatic.prerender( +
+ + + +
, + {signal: controller.signal, onError() {}}, + ); + await jest.runAllTimers(); + expect(lifetimes).toHaveLength(1); + expect(lifetimes[0].aborted).toBe(false); + + controller.abort(); + hasLoaded = true; + resolve(); + await serverAct(() => resultPromise); + + expect(lifetimes[0].aborted).toBe(true); + }); + + it('detaches the listener when the stream is cancelled', async () => { + let hasLoaded = false; + let resolve; + const promise = new Promise(r => (resolve = r)); + function Wait() { + if (!hasLoaded) { + throw promise; + } + return 'Done'; + } + + const controller = new AbortController(); + const lifetimes = trackAbortListenerLifetimes(controller.signal); + + const stream = await serverAct(() => + ReactDOMFizzServer.renderToReadableStream( +
+ + + +
, + {signal: controller.signal, onError() {}}, + ), + ); + expect(lifetimes).toHaveLength(1); + expect(lifetimes[0].aborted).toBe(false); + + await serverAct(() => stream.cancel()); + hasLoaded = true; + resolve(); + + expect(lifetimes[0].aborted).toBe(true); + }); + + it('detaches the listener when the shell errors', async () => { + // A shell error rejects before the caller ever receives a stream, so + // nothing consumes the request and it never closes. The listener has to + // come off at the fatal error itself. + function Boom() { + throw new Error('boom'); + } + + const controller = new AbortController(); + const lifetimes = trackAbortListenerLifetimes(controller.signal); + + await expect( + serverAct(() => + ReactDOMFizzServer.renderToReadableStream(, { + signal: controller.signal, + onError() {}, + }), + ), + ).rejects.toThrow('boom'); + + expect(lifetimes).toHaveLength(1); + expect(lifetimes[0].aborted).toBe(true); + }); + + it('attaches no listener when the signal is already aborted', async () => { + const controller = new AbortController(); + controller.abort(); + const lifetimes = trackAbortListenerLifetimes(controller.signal); + + await serverAct(() => + ReactDOMFizzStatic.prerender(
hello world
, { + signal: controller.signal, + onError() {}, + }), + ); + + expect(lifetimes).toHaveLength(0); + }); + }); }); diff --git a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js index 162e937f96d..472f5296e1c 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js @@ -24,6 +24,7 @@ import { startFlowing, stopFlowing, abort, + attachAbortSignal, } from 'react-server/src/ReactFizzServer'; import { @@ -151,16 +152,7 @@ function renderToReadableStream( options ? options.formState : undefined, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); @@ -221,16 +213,7 @@ function resume( onFatalError, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-dom/src/server/ReactDOMFizzServerBun.js b/packages/react-dom/src/server/ReactDOMFizzServerBun.js index 8e3ce7a411c..10186318de2 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerBun.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerBun.js @@ -23,6 +23,7 @@ import { startFlowing, stopFlowing, abort, + attachAbortSignal, } from 'react-server/src/ReactFizzServer'; import { @@ -140,16 +141,7 @@ function renderToReadableStream( options ? options.formState : undefined, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-dom/src/server/ReactDOMFizzServerEdge.js b/packages/react-dom/src/server/ReactDOMFizzServerEdge.js index 162e937f96d..472f5296e1c 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerEdge.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerEdge.js @@ -24,6 +24,7 @@ import { startFlowing, stopFlowing, abort, + attachAbortSignal, } from 'react-server/src/ReactFizzServer'; import { @@ -151,16 +152,7 @@ function renderToReadableStream( options ? options.formState : undefined, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); @@ -221,16 +213,7 @@ function resume( onFatalError, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-dom/src/server/ReactDOMFizzServerNode.js b/packages/react-dom/src/server/ReactDOMFizzServerNode.js index d99dcfef405..65edf9bcfba 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerNode.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerNode.js @@ -30,6 +30,7 @@ import { startFlowing, stopFlowing, abort, + attachAbortSignal, prepareForStartFlowingIfBeforeAllReady, } from 'react-server/src/ReactFizzServer'; @@ -289,16 +290,7 @@ function renderToReadableStream( options ? options.formState : undefined, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); @@ -427,16 +419,7 @@ function resume( onFatalError, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js b/packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js index 91c5e59384c..22233ec06bb 100644 --- a/packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js +++ b/packages/react-dom/src/server/ReactDOMFizzStaticBrowser.js @@ -24,6 +24,7 @@ import { startFlowing, stopFlowing, abort, + attachAbortSignal, getPostponedState, } from 'react-server/src/ReactFizzServer'; @@ -132,16 +133,7 @@ function prerender( onFatalError, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); @@ -199,16 +191,7 @@ function resumeAndPrerender( onFatalError, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-dom/src/server/ReactDOMFizzStaticEdge.js b/packages/react-dom/src/server/ReactDOMFizzStaticEdge.js index 0117123523a..39d68db37ba 100644 --- a/packages/react-dom/src/server/ReactDOMFizzStaticEdge.js +++ b/packages/react-dom/src/server/ReactDOMFizzStaticEdge.js @@ -24,6 +24,7 @@ import { startFlowing, stopFlowing, abort, + attachAbortSignal, getPostponedState, } from 'react-server/src/ReactFizzServer'; @@ -131,16 +132,7 @@ function prerender( onFatalError, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); @@ -197,16 +189,7 @@ function resumeAndPrerender( onFatalError, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-dom/src/server/ReactDOMFizzStaticNode.js b/packages/react-dom/src/server/ReactDOMFizzStaticNode.js index 7aabc76608b..39253373287 100644 --- a/packages/react-dom/src/server/ReactDOMFizzStaticNode.js +++ b/packages/react-dom/src/server/ReactDOMFizzStaticNode.js @@ -26,6 +26,7 @@ import { startFlowing, stopFlowing, abort, + attachAbortSignal, getPostponedState, } from 'react-server/src/ReactFizzServer'; @@ -163,16 +164,7 @@ function prerenderToNodeStream( onFatalError, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); @@ -254,16 +246,7 @@ function prerender( onFatalError, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); @@ -310,16 +293,7 @@ function resumeAndPrerenderToNodeStream( onFatalError, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); @@ -377,16 +351,7 @@ function resumeAndPrerender( onFatalError, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-flight-server-fb/src/client/ReactFlightDOMClientBrowser.js b/packages/react-flight-server-fb/src/client/ReactFlightDOMClientBrowser.js index 51bca925c84..4df0fa9fc3a 100644 --- a/packages/react-flight-server-fb/src/client/ReactFlightDOMClientBrowser.js +++ b/packages/react-flight-server-fb/src/client/ReactFlightDOMClientBrowser.js @@ -264,7 +264,7 @@ function encodeReply( string | URLSearchParams | FormData, > /* We don't use URLSearchParams yet but maybe */ { return new Promise((resolve, reject) => { - const abort = processReply( + processReply( value, '', options && options.temporaryReferences @@ -272,19 +272,8 @@ function encodeReply( : undefined, resolve, reject, + options ? options.signal : undefined, ); - if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort((signal as any).reason); - } else { - const listener = () => { - abort((signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } - } }); } diff --git a/packages/react-markup/src/ReactMarkupClient.js b/packages/react-markup/src/ReactMarkupClient.js index 3c70e21e003..ee72673e124 100644 --- a/packages/react-markup/src/ReactMarkupClient.js +++ b/packages/react-markup/src/ReactMarkupClient.js @@ -16,7 +16,7 @@ import { createRequest as createFizzRequest, startWork as startFizzWork, startFlowing as startFizzFlowing, - abort as abortFizz, + attachAbortSignal as attachFizzAbortSignal, } from 'react-server/src/ReactFizzServer'; import { @@ -88,16 +88,7 @@ export function experimental_renderToHTML( undefined, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abortFizz(fizzRequest, (signal as any).reason); - } else { - const listener = () => { - abortFizz(fizzRequest, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachFizzAbortSignal(fizzRequest, options.signal); } startFizzWork(fizzRequest); startFizzFlowing(fizzRequest, fizzDestination); diff --git a/packages/react-markup/src/ReactMarkupServer.js b/packages/react-markup/src/ReactMarkupServer.js index 15a2e7827c1..b47ee75aca1 100644 --- a/packages/react-markup/src/ReactMarkupServer.js +++ b/packages/react-markup/src/ReactMarkupServer.js @@ -21,6 +21,7 @@ import { startWork as startFlightWork, startFlowing as startFlightFlowing, abort as abortFlight, + attachAbortSignal as attachFlightAbortSignal, } from 'react-server/src/ReactFlightServer'; import { @@ -36,6 +37,7 @@ import { startWork as startFizzWork, startFlowing as startFizzFlowing, abort as abortFizz, + attachAbortSignal as attachFizzAbortSignal, } from 'react-server/src/ReactFizzServer'; import { @@ -217,19 +219,10 @@ export function experimental_renderToHTML( undefined, undefined, ); - if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abortFlight(flightRequest, (signal as any).reason); - abortFizz(fizzRequest, (signal as any).reason); - } else { - const listener = () => { - abortFlight(flightRequest, (signal as any).reason); - abortFizz(fizzRequest, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + const signal = options ? options.signal : undefined; + if (signal) { + attachFlightAbortSignal(flightRequest, signal); + attachFizzAbortSignal(fizzRequest, signal); } startFlightWork(flightRequest); startFlightFlowing(flightRequest, flightDestination); diff --git a/packages/react-noop-renderer/src/ReactNoopFlightServer.js b/packages/react-noop-renderer/src/ReactNoopFlightServer.js index 1bd96808508..f05d9d94d75 100644 --- a/packages/react-noop-renderer/src/ReactNoopFlightServer.js +++ b/packages/react-noop-renderer/src/ReactNoopFlightServer.js @@ -96,15 +96,7 @@ function render(model: ReactClientValue, options?: Options): Destination { ); const signal = options ? options.signal : undefined; if (signal) { - if (signal.aborted) { - ReactNoopFlightServer.abort(request, (signal as any).reason); - } else { - const listener = () => { - ReactNoopFlightServer.abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + ReactNoopFlightServer.attachAbortSignal(request, signal); } if (__DEV__ && options && options.debugChannel !== undefined) { options.debugChannel.onMessage = message => { diff --git a/packages/react-server-dom-esm/src/client/ReactFlightDOMClientBrowser.js b/packages/react-server-dom-esm/src/client/ReactFlightDOMClientBrowser.js index 5029c092a79..9e81000b529 100644 --- a/packages/react-server-dom-esm/src/client/ReactFlightDOMClientBrowser.js +++ b/packages/react-server-dom-esm/src/client/ReactFlightDOMClientBrowser.js @@ -265,7 +265,7 @@ function encodeReply( string | URLSearchParams | FormData, > /* We don't use URLSearchParams yet but maybe */ { return new Promise((resolve, reject) => { - const abort = processReply( + processReply( value, '', options && options.temporaryReferences @@ -273,19 +273,8 @@ function encodeReply( : undefined, resolve, reject, + options ? options.signal : undefined, ); - if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort((signal as any).reason); - } else { - const listener = () => { - abort((signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } - } }); } diff --git a/packages/react-server-dom-esm/src/server/ReactFlightDOMServerNode.js b/packages/react-server-dom-esm/src/server/ReactFlightDOMServerNode.js index acdbe46e56e..5f5b8f68f74 100644 --- a/packages/react-server-dom-esm/src/server/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-esm/src/server/ReactFlightDOMServerNode.js @@ -30,6 +30,7 @@ import { startFlowingDebug, stopFlowing, abort, + attachAbortSignal, resolveDebugMessage, closeDebugChannel, } from 'react-server/src/ReactFlightServer'; @@ -313,18 +314,7 @@ function prerenderToNodeStream( false, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - const reason = (signal as any).reason; - abort(request, reason); - } else { - const listener = () => { - const reason = (signal as any).reason; - abort(request, reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-server-dom-parcel/src/client/ReactFlightDOMClientBrowser.js b/packages/react-server-dom-parcel/src/client/ReactFlightDOMClientBrowser.js index 9faafe2ea62..4bf62476049 100644 --- a/packages/react-server-dom-parcel/src/client/ReactFlightDOMClientBrowser.js +++ b/packages/react-server-dom-parcel/src/client/ReactFlightDOMClientBrowser.js @@ -299,7 +299,7 @@ export function encodeReply( string | URLSearchParams | FormData, > /* We don't use URLSearchParams yet but maybe */ { return new Promise((resolve, reject) => { - const abort = processReply( + processReply( value, '', // formFieldPrefix options && options.temporaryReferences @@ -307,19 +307,8 @@ export function encodeReply( : undefined, resolve, reject, + options ? options.signal : undefined, ); - if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort((signal as any).reason); - } else { - const listener = () => { - abort((signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } - } }); } diff --git a/packages/react-server-dom-parcel/src/client/ReactFlightDOMClientEdge.js b/packages/react-server-dom-parcel/src/client/ReactFlightDOMClientEdge.js index ab0f5c02ded..71cfda89ffe 100644 --- a/packages/react-server-dom-parcel/src/client/ReactFlightDOMClientEdge.js +++ b/packages/react-server-dom-parcel/src/client/ReactFlightDOMClientEdge.js @@ -230,7 +230,7 @@ export function encodeReply( string | URLSearchParams | FormData, > /* We don't use URLSearchParams yet but maybe */ { return new Promise((resolve, reject) => { - const abort = processReply( + processReply( value, '', options && options.temporaryReferences @@ -238,18 +238,7 @@ export function encodeReply( : undefined, resolve, reject, + options ? options.signal : undefined, ); - if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort((signal as any).reason); - } else { - const listener = () => { - abort((signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } - } }); } diff --git a/packages/react-server-dom-parcel/src/server/ReactFlightDOMServerBrowser.js b/packages/react-server-dom-parcel/src/server/ReactFlightDOMServerBrowser.js index 8476fcafe2a..97e841ef1d7 100644 --- a/packages/react-server-dom-parcel/src/server/ReactFlightDOMServerBrowser.js +++ b/packages/react-server-dom-parcel/src/server/ReactFlightDOMServerBrowser.js @@ -29,6 +29,7 @@ import { startFlowingDebug, stopFlowing, abort, + attachAbortSignal, resolveDebugMessage, closeDebugChannel, } from 'react-server/src/ReactFlightServer'; @@ -136,16 +137,7 @@ export function renderToReadableStream( debugChannelReadable !== undefined, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } if (debugChannelWritable !== undefined) { const debugStream = new ReadableStream( @@ -227,18 +219,7 @@ export function prerender( false, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - const reason = (signal as any).reason; - abort(request, reason); - } else { - const listener = () => { - const reason = (signal as any).reason; - abort(request, reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-server-dom-parcel/src/server/ReactFlightDOMServerEdge.js b/packages/react-server-dom-parcel/src/server/ReactFlightDOMServerEdge.js index 6100f4eb903..4b22b92cacf 100644 --- a/packages/react-server-dom-parcel/src/server/ReactFlightDOMServerEdge.js +++ b/packages/react-server-dom-parcel/src/server/ReactFlightDOMServerEdge.js @@ -32,6 +32,7 @@ import { startFlowingDebug, stopFlowing, abort, + attachAbortSignal, resolveDebugMessage, closeDebugChannel, } from 'react-server/src/ReactFlightServer'; @@ -142,16 +143,7 @@ export function renderToReadableStream( debugChannelReadable !== undefined, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } if (debugChannelWritable !== undefined) { const debugStream = new ReadableStream( @@ -233,18 +225,7 @@ export function prerender( false, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - const reason = (signal as any).reason; - abort(request, reason); - } else { - const listener = () => { - const reason = (signal as any).reason; - abort(request, reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-server-dom-parcel/src/server/ReactFlightDOMServerNode.js b/packages/react-server-dom-parcel/src/server/ReactFlightDOMServerNode.js index 395727f69cf..33d1b3708ab 100644 --- a/packages/react-server-dom-parcel/src/server/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-parcel/src/server/ReactFlightDOMServerNode.js @@ -35,6 +35,7 @@ import { startFlowingDebug, stopFlowing, abort, + attachAbortSignal, resolveDebugMessage, closeDebugChannel, } from 'react-server/src/ReactFlightServer'; @@ -362,16 +363,7 @@ export function renderToReadableStream( debugChannelReadable !== undefined, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } if (debugChannelWritable !== undefined) { let debugWritable: Writable; @@ -478,18 +470,7 @@ export function prerenderToNodeStream( false, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - const reason = (signal as any).reason; - abort(request, reason); - } else { - const listener = () => { - const reason = (signal as any).reason; - abort(request, reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); @@ -542,18 +523,7 @@ export function prerender( false, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - const reason = (signal as any).reason; - abort(request, reason); - } else { - const listener = () => { - const reason = (signal as any).reason; - abort(request, reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-server-dom-turbopack/src/client/ReactFlightDOMClientBrowser.js b/packages/react-server-dom-turbopack/src/client/ReactFlightDOMClientBrowser.js index c42728a9f9e..34bc706125d 100644 --- a/packages/react-server-dom-turbopack/src/client/ReactFlightDOMClientBrowser.js +++ b/packages/react-server-dom-turbopack/src/client/ReactFlightDOMClientBrowser.js @@ -265,7 +265,7 @@ function encodeReply( string | URLSearchParams | FormData, > /* We don't use URLSearchParams yet but maybe */ { return new Promise((resolve, reject) => { - const abort = processReply( + processReply( value, '', options && options.temporaryReferences @@ -273,19 +273,8 @@ function encodeReply( : undefined, resolve, reject, + options ? options.signal : undefined, ); - if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort((signal as any).reason); - } else { - const listener = () => { - abort((signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } - } }); } diff --git a/packages/react-server-dom-turbopack/src/client/ReactFlightDOMClientEdge.js b/packages/react-server-dom-turbopack/src/client/ReactFlightDOMClientEdge.js index 314d6e125b6..71298014c47 100644 --- a/packages/react-server-dom-turbopack/src/client/ReactFlightDOMClientEdge.js +++ b/packages/react-server-dom-turbopack/src/client/ReactFlightDOMClientEdge.js @@ -232,7 +232,7 @@ function encodeReply( string | URLSearchParams | FormData, > /* We don't use URLSearchParams yet but maybe */ { return new Promise((resolve, reject) => { - const abort = processReply( + processReply( value, '', options && options.temporaryReferences @@ -240,19 +240,8 @@ function encodeReply( : undefined, resolve, reject, + options ? options.signal : undefined, ); - if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort((signal as any).reason); - } else { - const listener = () => { - abort((signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } - } }); } diff --git a/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerBrowser.js b/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerBrowser.js index dca1b364fb4..84bd3d6f290 100644 --- a/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerBrowser.js +++ b/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerBrowser.js @@ -23,6 +23,7 @@ import { startFlowingDebug, stopFlowing, abort, + attachAbortSignal, resolveDebugMessage, closeDebugChannel, } from 'react-server/src/ReactFlightServer'; @@ -133,16 +134,7 @@ function renderToReadableStream( debugChannelReadable !== undefined, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } if (debugChannelWritable !== undefined) { const debugStream = new ReadableStream( @@ -225,18 +217,7 @@ function prerender( false, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - const reason = (signal as any).reason; - abort(request, reason); - } else { - const listener = () => { - const reason = (signal as any).reason; - abort(request, reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerEdge.js b/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerEdge.js index 524944c1fb1..e92697ed5d4 100644 --- a/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerEdge.js +++ b/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerEdge.js @@ -26,6 +26,7 @@ import { startFlowingDebug, stopFlowing, abort, + attachAbortSignal, resolveDebugMessage, closeDebugChannel, } from 'react-server/src/ReactFlightServer'; @@ -139,16 +140,7 @@ function renderToReadableStream( debugChannelReadable !== undefined, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } if (debugChannelWritable !== undefined) { const debugStream = new ReadableStream( @@ -231,18 +223,7 @@ function prerender( false, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - const reason = (signal as any).reason; - abort(request, reason); - } else { - const listener = () => { - const reason = (signal as any).reason; - abort(request, reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerNode.js b/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerNode.js index 85a48641326..2c5c987a70b 100644 --- a/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-turbopack/src/server/ReactFlightDOMServerNode.js @@ -33,6 +33,7 @@ import { startFlowingDebug, stopFlowing, abort, + attachAbortSignal, resolveDebugMessage, closeDebugChannel, } from 'react-server/src/ReactFlightServer'; @@ -357,16 +358,7 @@ function renderToReadableStream( debugChannelReadable !== undefined, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } if (debugChannelWritable !== undefined) { let debugWritable: Writable; @@ -474,18 +466,7 @@ function prerenderToNodeStream( false, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - const reason = (signal as any).reason; - abort(request, reason); - } else { - const listener = () => { - const reason = (signal as any).reason; - abort(request, reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); @@ -539,18 +520,7 @@ function prerender( false, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - const reason = (signal as any).reason; - abort(request, reason); - } else { - const listener = () => { - const reason = (signal as any).reason; - abort(request, reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-server-dom-unbundled/src/client/ReactFlightDOMClientEdge.js b/packages/react-server-dom-unbundled/src/client/ReactFlightDOMClientEdge.js index 2ccbd6a1d67..2cbb9b307e6 100644 --- a/packages/react-server-dom-unbundled/src/client/ReactFlightDOMClientEdge.js +++ b/packages/react-server-dom-unbundled/src/client/ReactFlightDOMClientEdge.js @@ -232,7 +232,7 @@ function encodeReply( string | URLSearchParams | FormData, > /* We don't use URLSearchParams yet but maybe */ { return new Promise((resolve, reject) => { - const abort = processReply( + processReply( value, '', options && options.temporaryReferences @@ -240,19 +240,8 @@ function encodeReply( : undefined, resolve, reject, + options ? options.signal : undefined, ); - if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort((signal as any).reason); - } else { - const listener = () => { - abort((signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } - } }); } diff --git a/packages/react-server-dom-unbundled/src/server/ReactFlightDOMServerNode.js b/packages/react-server-dom-unbundled/src/server/ReactFlightDOMServerNode.js index e9edbcbdbb8..03fe8d6094a 100644 --- a/packages/react-server-dom-unbundled/src/server/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-unbundled/src/server/ReactFlightDOMServerNode.js @@ -33,6 +33,7 @@ import { startFlowingDebug, stopFlowing, abort, + attachAbortSignal, resolveDebugMessage, closeDebugChannel, } from 'react-server/src/ReactFlightServer'; @@ -357,16 +358,7 @@ function renderToReadableStream( debugChannelReadable !== undefined, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } if (debugChannelWritable !== undefined) { let debugWritable: Writable; @@ -474,18 +466,7 @@ function prerenderToNodeStream( false, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - const reason = (signal as any).reason; - abort(request, reason); - } else { - const listener = () => { - const reason = (signal as any).reason; - abort(request, reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); @@ -539,18 +520,7 @@ function prerender( false, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - const reason = (signal as any).reason; - abort(request, reason); - } else { - const listener = () => { - const reason = (signal as any).reason; - abort(request, reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js index 9a8b2736f2d..d382cf6864b 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js @@ -3082,6 +3082,85 @@ describe('ReactFlightDOMBrowser', () => { } }); + it('should abort the cache signal when a render completes while debug objects are still retained', async () => { + // A debug channel with a readable side lets the client fetch debug objects + // lazily. React serializes each component's props into the debug model, and + // it defers the part of an object tree that exceeds the model's object + // limit. A deferred object stays retained, and its debug chunk stays + // pending, until the client asks for it or the channel closes. The render + // below finishes while one such object is outstanding. + function createDeepJSX(n) { + if (n <= 0) { + return null; + } + return
{createDeepJSX(n - 1)}
; + } + + let cacheSignal; + + function ServerComponent() { + cacheSignal = ReactServer.cacheSignal(); + return
not using props
; + } + + let debugChannelReadableController; + const debugChunks = []; + + const debugChannelReadable = new ReadableStream({ + start(controller) { + debugChannelReadableController = controller; + }, + }); + + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + // These children nest deeper than the debug model's object limit. + {createDeepJSX(20)}, + webpackMap, + { + debugChannel: { + readable: debugChannelReadable, + writable: new WritableStream({ + write(chunk) { + debugChunks.push(chunk); + }, + }), + }, + }, + ), + ); + + const reader = stream.getReader(); + while (true) { + const {done} = await reader.read(); + if (done) { + break; + } + } + await serverAct(() => {}); + + if (__DEV__) { + // Fail loudly if the setup stops deferring anything, for example because + // the object limit changed. Without a retained object this test passes + // for the wrong reason. + const debugOutput = debugChunks + .map(chunk => new TextDecoder().decode(chunk)) + .join(''); + expect(debugOutput).toContain('$Y'); + } + + expect(cacheSignal.aborted).toBe(true); + + // Closing the debug channel drops the retained objects. The signal must + // already be aborted at that point, and must stay aborted. + await serverAct(() => { + debugChannelReadableController.close(); + }); + await serverAct(() => {}); + + expect(cacheSignal.aborted).toBe(true); + }); + it('should resolve a cycle between debug info and the value it produces when using a debug channel', async () => { // Same as `should resolve a cycle between debug info and the value it produces`, but using a debug channel. @@ -3359,6 +3438,152 @@ describe('ReactFlightDOMBrowser', () => { ); }); + describe('abort signal lifetime', () => { + // Collects the lifetime signal that React bounds each abort listener with. + // React passes that signal to addEventListener instead of calling + // removeEventListener, so the runtime performs the removal and nothing here + // observes it directly. An aborted lifetime is what shows the listener is + // gone. ReactFlightDOMNode-test asserts the removal itself, which needs a + // Node API that jsdom does not have. + function trackAbortListenerLifetimes(signal) { + const lifetimes = []; + const add = signal.addEventListener.bind(signal); + signal.addEventListener = (type, listener, options) => { + if (type === 'abort') { + lifetimes.push(options.signal); + } + return add(type, listener, options); + }; + return lifetimes; + } + + async function drain(stream) { + const reader = stream.getReader(); + while (true) { + const {done} = await reader.read(); + if (done) { + return; + } + } + } + + function App() { + return
hello world
; + } + + it('detaches the listener when a prerender completes', async () => { + const controller = new AbortController(); + const lifetimes = trackAbortListenerLifetimes(controller.signal); + + const {prelude} = await serverAct(() => + ReactServerDOMStaticServer.prerender(, webpackMap, { + signal: controller.signal, + }), + ); + expect(lifetimes).toHaveLength(1); + expect(lifetimes[0].aborted).toBe(false); + + await serverAct(() => drain(prelude)); + expect(lifetimes[0].aborted).toBe(true); + }); + + it('detaches the listener when a render completes', async () => { + const controller = new AbortController(); + const lifetimes = trackAbortListenerLifetimes(controller.signal); + + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(, webpackMap, { + signal: controller.signal, + }), + ); + expect(lifetimes).toHaveLength(1); + expect(lifetimes[0].aborted).toBe(false); + + await serverAct(() => drain(stream)); + expect(lifetimes[0].aborted).toBe(true); + }); + + it('detaches the listener when the signal aborts mid-render', async () => { + let resolveGreeting; + const greetingPromise = new Promise(resolve => { + resolveGreeting = resolve; + }); + + async function Greeting() { + await greetingPromise; + return 'hello world'; + } + + const controller = new AbortController(); + const lifetimes = trackAbortListenerLifetimes(controller.signal); + + const {pendingResult} = await serverAct(async () => { + return { + pendingResult: ReactServerDOMStaticServer.prerender( + , + webpackMap, + {signal: controller.signal, onError() {}}, + ), + }; + }); + expect(lifetimes).toHaveLength(1); + expect(lifetimes[0].aborted).toBe(false); + + controller.abort('boom'); + resolveGreeting(); + await serverAct(() => pendingResult); + + expect(lifetimes[0].aborted).toBe(true); + }); + + it('detaches the listener when the stream is cancelled', async () => { + let resolveGreeting; + const greetingPromise = new Promise(resolve => { + resolveGreeting = resolve; + }); + + async function Greeting() { + await greetingPromise; + return 'hello world'; + } + + const controller = new AbortController(); + const lifetimes = trackAbortListenerLifetimes(controller.signal); + + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream(, webpackMap, { + signal: controller.signal, + onError() {}, + }), + ); + expect(lifetimes).toHaveLength(1); + expect(lifetimes[0].aborted).toBe(false); + + await serverAct(() => stream.cancel('boom')); + resolveGreeting(); + + expect(lifetimes[0].aborted).toBe(true); + }); + + it('attaches no listener when the signal is already aborted', async () => { + const controller = new AbortController(); + controller.abort('boom'); + const lifetimes = trackAbortListenerLifetimes(controller.signal); + + await serverAct(() => + ReactServerDOMStaticServer.prerender(, webpackMap, { + signal: controller.signal, + onError() {}, + }), + ); + + expect(lifetimes).toHaveLength(0); + }); + + // The composite-signal case lives in ReactFlightDOMNode-test, because + // jsdom's AbortSignal has no AbortSignal.any. + }); + describe('with console.createTask', () => { // Stands in for what a browser console does with fake tasks: whatever runs // inside a task is shown under that task's name in the async stack. This is diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js index 9899d821956..88e3dcef2bf 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js @@ -2446,4 +2446,43 @@ describe('ReactFlightDOMNode', () => { expect(Buffer.isBuffer(result.font)).toBe(false); expect(result.font).toEqual({type: 'Buffer', data: [1, 2, 3, 4]}); }); + + it('detaches the abort listener from a composite signal once the prerender completes', async () => { + // A composite signal from AbortSignal.any() is retained by the runtime for + // as long as it has an abort listener attached, so a listener left behind + // by a completed render keeps that render reachable for the lifetime of the + // source signals. + // + // React bounds its listener with a lifetime signal, so the runtime removes + // the listener rather than React calling removeEventListener. This test + // observes the registration itself through a Node API, which is the only + // way to see that removal. The suites that run under jsdom assert on the + // lifetime signal instead. + const {getEventListeners} = require('node:events'); + + const outer = new AbortController(); + const timeout = new AbortController(); + const composite = AbortSignal.any([outer.signal, timeout.signal]); + + function App() { + return
hello world
; + } + + const {prelude} = await serverAct(() => + ReactServerDOMStaticServer.prerenderToNodeStream(, webpackMap, { + signal: composite, + }), + ); + expect(getEventListeners(composite, 'abort')).toHaveLength(1); + + await serverAct( + () => + new Promise(resolve => { + prelude.resume(); + prelude.on('end', resolve); + }), + ); + + expect(getEventListeners(composite, 'abort')).toHaveLength(0); + }); }); diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReply-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReply-test.js index a13a55c36f1..9d9eee434e6 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReply-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMReply-test.js @@ -755,4 +755,49 @@ describe('ReactFlightDOMReply', () => { } expect(error.message).toContain('Referenced Blob is not a Blob.'); }); + + it('detaches the abort listener once the reply is encoded', async () => { + const controller = new AbortController(); + const signal = controller.signal; + + // Collects the lifetime signal that React bounds each abort listener with. + // React passes that signal to addEventListener instead of calling + // removeEventListener, so the runtime performs the removal and nothing here + // observes it directly. An aborted lifetime is what shows the listener is + // gone. + const lifetimes = []; + const add = signal.addEventListener.bind(signal); + signal.addEventListener = (type, listener, options) => { + if (type === 'abort') { + lifetimes.push(options.signal); + } + return add(type, listener, options); + }; + + let resolvePart; + const part = new Promise(r => (resolvePart = r)); + const bodyPromise = ReactServerDOMClient.encodeReply( + {part, hello: 'world'}, + {signal}, + ); + expect(lifetimes).toHaveLength(1); + expect(lifetimes[0].aborted).toBe(false); + + resolvePart('done'); + await bodyPromise; + + expect(lifetimes[0].aborted).toBe(true); + }); + + it('resolves with the partial result when the signal is already aborted', async () => { + const controller = new AbortController(); + controller.abort(); + const neverResolves = new Promise(() => {}); + const body = await ReactServerDOMClient.encodeReply( + {promise: neverResolves, hello: 'world'}, + {signal: controller.signal}, + ); + const result = await ReactServerDOMServer.decodeReply(body); + expect(result.hello).toBe('world'); + }); }); diff --git a/packages/react-server-dom-webpack/src/client/ReactFlightDOMClientBrowser.js b/packages/react-server-dom-webpack/src/client/ReactFlightDOMClientBrowser.js index c42728a9f9e..34bc706125d 100644 --- a/packages/react-server-dom-webpack/src/client/ReactFlightDOMClientBrowser.js +++ b/packages/react-server-dom-webpack/src/client/ReactFlightDOMClientBrowser.js @@ -265,7 +265,7 @@ function encodeReply( string | URLSearchParams | FormData, > /* We don't use URLSearchParams yet but maybe */ { return new Promise((resolve, reject) => { - const abort = processReply( + processReply( value, '', options && options.temporaryReferences @@ -273,19 +273,8 @@ function encodeReply( : undefined, resolve, reject, + options ? options.signal : undefined, ); - if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort((signal as any).reason); - } else { - const listener = () => { - abort((signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } - } }); } diff --git a/packages/react-server-dom-webpack/src/client/ReactFlightDOMClientEdge.js b/packages/react-server-dom-webpack/src/client/ReactFlightDOMClientEdge.js index 2ccbd6a1d67..2cbb9b307e6 100644 --- a/packages/react-server-dom-webpack/src/client/ReactFlightDOMClientEdge.js +++ b/packages/react-server-dom-webpack/src/client/ReactFlightDOMClientEdge.js @@ -232,7 +232,7 @@ function encodeReply( string | URLSearchParams | FormData, > /* We don't use URLSearchParams yet but maybe */ { return new Promise((resolve, reject) => { - const abort = processReply( + processReply( value, '', options && options.temporaryReferences @@ -240,19 +240,8 @@ function encodeReply( : undefined, resolve, reject, + options ? options.signal : undefined, ); - if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort((signal as any).reason); - } else { - const listener = () => { - abort((signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } - } }); } diff --git a/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerBrowser.js b/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerBrowser.js index 5ea5f21dd11..1fee811b27a 100644 --- a/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerBrowser.js +++ b/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerBrowser.js @@ -23,6 +23,7 @@ import { startFlowingDebug, stopFlowing, abort, + attachAbortSignal, resolveDebugMessage, closeDebugChannel, } from 'react-server/src/ReactFlightServer'; @@ -133,16 +134,7 @@ function renderToReadableStream( debugChannelReadable !== undefined, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } if (debugChannelWritable !== undefined) { const debugStream = new ReadableStream( @@ -225,18 +217,7 @@ function prerender( false, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - const reason = (signal as any).reason; - abort(request, reason); - } else { - const listener = () => { - const reason = (signal as any).reason; - abort(request, reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerEdge.js b/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerEdge.js index 22bb0656d72..ba79e18b73d 100644 --- a/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerEdge.js +++ b/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerEdge.js @@ -26,6 +26,7 @@ import { startFlowingDebug, stopFlowing, abort, + attachAbortSignal, resolveDebugMessage, closeDebugChannel, } from 'react-server/src/ReactFlightServer'; @@ -139,16 +140,7 @@ function renderToReadableStream( debugChannelReadable !== undefined, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } if (debugChannelWritable !== undefined) { const debugStream = new ReadableStream( @@ -231,18 +223,7 @@ function prerender( false, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - const reason = (signal as any).reason; - abort(request, reason); - } else { - const listener = () => { - const reason = (signal as any).reason; - abort(request, reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerNode.js b/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerNode.js index fcf32c342e7..12bc590b909 100644 --- a/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-webpack/src/server/ReactFlightDOMServerNode.js @@ -33,6 +33,7 @@ import { startFlowingDebug, stopFlowing, abort, + attachAbortSignal, resolveDebugMessage, closeDebugChannel, } from 'react-server/src/ReactFlightServer'; @@ -357,16 +358,7 @@ function renderToReadableStream( debugChannelReadable !== undefined, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - abort(request, (signal as any).reason); - } else { - const listener = () => { - abort(request, (signal as any).reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } if (debugChannelWritable !== undefined) { let debugWritable: Writable; @@ -474,18 +466,7 @@ function prerenderToNodeStream( false, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - const reason = (signal as any).reason; - abort(request, reason); - } else { - const listener = () => { - const reason = (signal as any).reason; - abort(request, reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); @@ -539,18 +520,7 @@ function prerender( false, ); if (options && options.signal) { - const signal = options.signal; - if (signal.aborted) { - const reason = (signal as any).reason; - abort(request, reason); - } else { - const listener = () => { - const reason = (signal as any).reason; - abort(request, reason); - signal.removeEventListener('abort', listener); - }; - signal.addEventListener('abort', listener); - } + attachAbortSignal(request, options.signal); } startWork(request); }); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 8ea7013d17f..a863863ea60 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -376,6 +376,12 @@ const CLOSING = 12; const CLOSED = 13; const STALLED_DEV = 14; +// Passed to renderLifetimeController.abort(). Nothing reads the reason, but a +// call to abort() without one constructs an AbortError DOMException. Capturing +// the stack trace dominates that cost, and the cost grows with the depth of the +// stack. +const RENDER_ENDED = 'The render ended.'; + export opaque type Request = { destination: null | Destination, flushScheduled: boolean, @@ -427,6 +433,9 @@ export opaque type Request = { // emit a different response to the stream instead. onShellError: (error: mixed) => void, onFatalError: (error: mixed) => void, + // Aborted once the render ends, whether it completed, failed fatally or was + // aborted. Bounds the lifetime of anything that must not outlive the render. + renderLifetimeController: AbortController, // Form state that was the result of an MPA submission, if it was provided. formState: null | ReactFormState, // DEV-only, warning dedupe @@ -580,6 +589,7 @@ function RequestInstance( this.onShellReady = onShellReady === undefined ? noop : onShellReady; this.onShellError = onShellError === undefined ? noop : onShellError; this.onFatalError = onFatalError === undefined ? noop : onFatalError; + this.renderLifetimeController = new AbortController(); this.formState = formState === undefined ? null : formState; if (__DEV__) { this.didWarnForKey = null; @@ -1445,6 +1455,7 @@ function fatalError( } onFatalError(error); } + request.renderLifetimeController.abort(RENDER_ENDED); if (request.destination !== null) { request.status = CLOSED; closeWithError(request.destination, error); @@ -6341,6 +6352,7 @@ function flushCompletedQueues( } } // We're done. + request.renderLifetimeController.abort(RENDER_ENDED); request.status = CLOSED; close(destination); // We need to stop flowing now because we do not want any async contexts which might call @@ -6504,6 +6516,32 @@ function finishAbort(request: Request, abortableTasks: Set): void { } } +// Aborts the request when the caller's signal aborts. The render lifetime +// bounds the listener, so the runtime removes the listener as soon as the +// render ends. From that point on abort() returns early, so the listener has +// nothing left to do. +// +// The listener has to be removed, because it would otherwise keep the whole +// Request reachable for as long as the caller's signal lives. A composite +// signal from AbortSignal.any() is itself retained by the runtime while it has +// any abort listener attached. +// +// A request whose stream is neither consumed nor cancelled never ends, so its +// listener stays attached for as long as the caller's signal lives. +export function attachAbortSignal(request: Request, signal: AbortSignal): void { + if (signal.aborted) { + abort(request, signal.reason); + return; + } + signal.addEventListener( + 'abort', + () => { + abort(request, signal.reason); + }, + {signal: request.renderLifetimeController.signal}, + ); +} + // This is called to early terminate a request. It puts all pending boundaries in client rendered state. export function abort(request: Request, reason: mixed): void { if ( @@ -6514,6 +6552,7 @@ export function abort(request: Request, reason: mixed): void { // can be aborted. in practice this makes abort callable at most once per render. return; } + request.renderLifetimeController.abort(RENDER_ENDED); const isRecoverableReason = typeof reason === 'object' && reason !== null && diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 2220e2a5bff..d9177cd2764 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -6488,6 +6488,25 @@ function flushCompletedChunks(request: Request): void { flushBuffered(destination); } if (request.pendingChunks === 0) { + // There are no pending chunks left, so the render is complete and its cache + // signal is aborted here. Debug chunks can still be pending, but they carry + // development-only instrumentation rather than the render's output. + // + // This runs before the stream bookkeeping below, because that bookkeeping + // can close the main stream and set the status to CLOSED while debug chunks + // are outstanding. The abort only happens below ABORTING, so a later flush + // would skip it. Repeated flushes are safe, because aborting an aborted + // controller does nothing a second time. + // + // The taint queue stays untouched here. Debug chunks are checked against + // the taint registry as they are written, and a deferred debug object can + // be written long after this point. + if (request.status < ABORTING) { + const abortReason = new Error( + 'This render completed successfully. All cacheSignals are now aborted to allow clean up of any unused resources.', + ); + request.cacheController.abort(abortReason); + } if (__DEV__) { const debugDestination = request.debugDestination; if (request.pendingDebugChunks === 0) { @@ -6518,12 +6537,6 @@ function flushCompletedChunks(request: Request): void { if (enableTaint) { cleanupTaintQueue(request); } - if (request.status < ABORTING) { - const abortReason = new Error( - 'This render completed successfully. All cacheSignals are now aborted to allow clean up of any unused resources.', - ); - request.cacheController.abort(abortReason); - } if (request.destination !== null) { request.status = CLOSED; close(request.destination); @@ -6659,6 +6672,34 @@ function finishAbort( } } +// Aborts the request when the caller's signal aborts. The cache controller's +// signal bounds the listener's lifetime, so the runtime removes the listener as +// soon as that signal aborts. The cache controller aborts at every point that +// ends the render: a fatal error, the completion of the flush loop, and abort() +// itself. From any of those points on, abort() returns early, so the listener +// has nothing left to do. +// +// The listener has to be removed, because it would otherwise keep the whole +// Request reachable for as long as the caller's signal lives. A composite +// signal from AbortSignal.any() is itself retained by the runtime while it has +// any abort listener attached. +// +// A request whose stream is neither consumed nor cancelled never ends, so its +// listener stays attached for as long as the caller's signal lives. +export function attachAbortSignal(request: Request, signal: AbortSignal): void { + if (signal.aborted) { + abort(request, signal.reason); + return; + } + signal.addEventListener( + 'abort', + () => { + abort(request, signal.reason); + }, + {signal: request.cacheController.signal}, + ); +} + export function abort(request: Request, reason: mixed): void { // We define any status below OPEN as OPEN equivalent if (request.status > OPEN) {