fix(server): prevent RequestEvent retention in AsyncLocalStorage async resources after SSR

.changeset/fix-ssr-als-memory-leak.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: prevent RequestEvent retention in AsyncLocalStorage async resources after SSR

packages/kit/src/exports/internal/event.js

@@ -7,7 +7,9 @@ import { IN_WEBCONTAINER } from '../../runtime/server/constants.js';
 /** @type {RequestStore | null} */
 let sync_store = null;
 
-/** @type {AsyncLocalStorage<RequestStore | null> | null} */
+/** @typedef {{ current: RequestStore | null }} RequestStoreContainer */
+
+/** @type {AsyncLocalStorage<RequestStoreContainer | null> | null} */
 let als;
 
 import('node:async_hooks')
@@ -63,18 +65,54 @@ export function get_request_store() {
 }
 
 export function try_get_request_store() {
-	return sync_store ?? als?.getStore() ?? null;
+	return sync_store ?? als?.getStore()?.current ?? null;
 }
 
 /**
  * @template T
  * @param {RequestStore | null} store
  * @param {() => T} fn
+ * @param {boolean} [gc_barrier] - When true, nulls the ALS container after `fn` settles so that
+ *   async resources created during `fn` (e.g. Svelte 4 subscription callbacks) cannot retain the
+ *   RequestStore beyond the lifetime of the call. Only needed for the SSR render context where
+ *   long-lived subscriptions would otherwise pin the RequestEvent in memory.
+ *   See https://github.com/nodejs/node/issues/53408
  */
-export function with_request_store(store, fn) {
+export function with_request_store(store, fn, gc_barrier = false) {
 	try {
 		sync_store = store;
-		return als ? als.run(store, fn) : fn();
+		if (als) {
+			// Wrap the store in a container so that async resources created inside fn only hold a
+			// reference to the container object rather than the full RequestStore. When gc_barrier
+			// is true (SSR render path) we null container.current after fn settles, severing the
+			// only path from lingering async resources back to the RequestStore and allowing GC.
+			const container = /** @type {RequestStoreContainer} */ ({ current: store });
+			const result = als.run(container, fn);
+			if (
+				result !== null &&
+				typeof result === 'object' &&
+				typeof (/** @type {any} */ (result).then) === 'function'
+			) {
+				if (gc_barrier) {
+					return /** @type {T} */ (
+						/** @type {Promise<any>} */ (/** @type {unknown} */ (result)).then(
+							(value) => {
+								container.current = null;
+								return value;
+							},
+							(error) => {
+								container.current = null;
+								throw error;
+							}
+						)
+					);
+				}
+				return /** @type {T} */ (result);
+			}
+			if (gc_barrier) container.current = null;
+			return result;
+		}
+		return fn();
 	} finally {
 		// Since AsyncLocalStorage is not working in webcontainers, we don't reset `sync_store`
 		// and handle only one request at a time in `src/runtime/server/index.js`.

packages/kit/src/exports/internal/event.spec.js

@@ -0,0 +1,110 @@
+/** @import { RequestStore } from 'types' */
+import { describe, expect, test } from 'vitest';
+import { with_request_store, try_get_request_store, getRequestEvent } from './event.js';
+
+/** @returns {RequestStore} */
+function make_store() {
+	return {
+		event: /** @type {any} */ ({ url: 'http://localhost/' }),
+		state: /** @type {any} */ ({})
+	};
+}
+
+describe('with_request_store', () => {
+	test('exposes store via try_get_request_store during sync execution', () => {
+		const store = make_store();
+		let seen = null;
+		with_request_store(store, () => {
+			seen = try_get_request_store();
+		});
+		expect(seen).toBe(store);
+	});
+
+	test('exposes store via try_get_request_store during async execution', async () => {
+		const store = make_store();
+		let seen = null;
+		await with_request_store(store, async () => {
+			await Promise.resolve();
+			seen = try_get_request_store();
+		});
+		expect(seen).toBe(store);
+	});
+
+	test('returns null from try_get_request_store after sync execution completes', () => {
+		const store = make_store();
+		with_request_store(store, () => {});
+		expect(try_get_request_store()).toBeNull();
+	});
+
+	test('returns null from try_get_request_store after async execution completes', async () => {
+		const store = make_store();
+		// Use Promise.resolve() rather than async/await to return a thenable without
+		// triggering the require-await lint rule on a trivial async arrow.
+		await with_request_store(store, () => Promise.resolve('rendered'));
+		expect(try_get_request_store()).toBeNull();
+	});
+
+	// Regression test for https://github.com/sveltejs/kit/issues/15764
+	// Async resources created inside als.run() inherit kResourceStore and can retain the
+	// RequestStore after the render completes (Node.js AsyncLocalStorage leak). The fix wraps
+	// the store in a container object; when gc_barrier=true (the render path), the container is
+	// nulled after render, allowing GC.
+	test('async continuations created inside render cannot access store after render completes', async () => {
+		const store = make_store();
+		let store_seen_in_continuation = /** @type {RequestStore | null | undefined} */ (undefined);
+
+		// Simulate what Svelte 4's component_subscribe does: create a Promise-based
+		// callback inside the ALS context that outlives the render.
+		let late_callback_promise = Promise.resolve(); // typed; overwritten inside render fn
+
+		// gc_barrier=true mirrors the SSR render call in render.js
+		await with_request_store(
+			store,
+			async () => {
+				await Promise.resolve(); // ensure the async code path through with_request_store
+				late_callback_promise = new Promise((resolve) => setTimeout(resolve, 0)).then(() => {
+					// This runs inside the ALS context that was active when setTimeout was called
+					// (i.e. the context from als.run()). Without the fix, try_get_request_store()
+					// returns `store` here. With the fix, the container is nulled so it returns null.
+					store_seen_in_continuation = try_get_request_store();
+				});
+			},
+			true
+		);
+
+		await late_callback_promise;
+
+		expect(store_seen_in_continuation).toBeNull();
+	});
+
+	test('getRequestEvent returns event during execution', () => {
+		const store = make_store();
+		let event = null;
+		with_request_store(store, () => {
+			event = getRequestEvent();
+		});
+		expect(event).toBe(store.event);
+	});
+
+	test('getRequestEvent throws outside execution context', () => {
+		expect(() => getRequestEvent()).toThrowError('Can only read the current request event');
+	});
+
+	test('nested with_request_store calls use innermost store', async () => {
+		const outer = make_store();
+		const inner = make_store();
+		let seen_in_inner = null;
+		let seen_after_inner = null;
+
+		await with_request_store(outer, async () => {
+			await Promise.resolve(); // exercise the async path
+			with_request_store(inner, () => {
+				seen_in_inner = try_get_request_store();
+			});
+			seen_after_inner = try_get_request_store();
+		});
+
+		expect(seen_in_inner).toBe(inner);
+		expect(seen_after_inner).toBe(outer);
+	});
+});

packages/kit/src/runtime/server/page/render.js

@@ -224,42 +224,47 @@ export async function render_response({
 
 			const state = { ...event_state, is_in_render: true };
 
-			rendered = await with_request_store({ event, state }, async () => {
-				// use relative paths during rendering, so that the resulting HTML is as
-				// portable as possible, but reset afterwards
-				if (paths.relative) paths.override({ base, assets });
-
-				const maybe_promise = options.root.render(props, render_opts);
-				// We have to invoke .then eagerly here in order to kick off rendering: it's only starting on access,
-				// and `await maybe_promise` would eagerly access the .then property but call its function only after a tick, which is too late
-				// for the paths.reset() below and for any eager getRequestEvent() calls during rendering without AsyncLocalStorage available.
-				const rendered =
-					options.async && 'then' in maybe_promise
-						? /** @type {ReturnType<typeof options.root.render> & Promise<any>} */ (
-								maybe_promise
-							).then((r) => r)
-						: maybe_promise;
-
-				// TODO 3.0 remove options.async
-				if (options.async) {
-					// we reset this synchronously, rather than after async rendering is complete,
-					// to avoid cross-talk between requests. This is a breaking change for
-					// anyone who opts into async SSR, since `base` and `assets` will no
-					// longer be relative to the current pathname.
-					// TODO 3.0 remove `base` and `assets` in favour of `resolve(...)` and `asset(...)`
-					paths.reset();
-				}
+			rendered = await with_request_store(
+				{ event, state },
+				async () => {
+					// use relative paths during rendering, so that the resulting HTML is as
+					// portable as possible, but reset afterwards
+					if (paths.relative) paths.override({ base, assets });
+
+					const maybe_promise = options.root.render(props, render_opts);
+					// We have to invoke .then eagerly here in order to kick off rendering: it's only starting on access,
+					// and `await maybe_promise` would eagerly access the .then property but call its function only after a tick, which is too late
+					// for the paths.reset() below and for any eager getRequestEvent() calls during rendering without AsyncLocalStorage available.
+					const rendered =
+						options.async && 'then' in maybe_promise
+							? /** @type {ReturnType<typeof options.root.render> & Promise<any>} */ (
+									maybe_promise
+								).then((r) => r)
+							: maybe_promise;
+
+					// TODO 3.0 remove options.async
+					if (options.async) {
+						// we reset this synchronously, rather than after async rendering is complete,
+						// to avoid cross-talk between requests. This is a breaking change for
+						// anyone who opts into async SSR, since `base` and `assets` will no
+						// longer be relative to the current pathname.
+						// TODO 3.0 remove `base` and `assets` in favour of `resolve(...)` and `asset(...)`
+						paths.reset();
+					}
 
-				const { head, html, css, hashes } = /** @type {ReturnType<typeof options.root.render>} */ (
-					options.async ? await rendered : rendered
-				);
+					const { head, html, css, hashes } =
+						/** @type {ReturnType<typeof options.root.render>} */ (
+							options.async ? await rendered : rendered
+						);
 
-				if (hashes) {
-					csp.add_script_hashes(hashes.script);
-				}
+					if (hashes) {
+						csp.add_script_hashes(hashes.script);
+					}
 
-				return { head, html, css, hashes };
-			});
+					return { head, html, css, hashes };
+				},
+				true
+			);
 		} finally {
 			if (DEV) {
 				globalThis.fetch = fetch;