fix(solid/server): CLIENT_HOLE is an inert thenable, not a Promise — no per-request retry reactions accumulate (#3657) (#3658)

Author: ryansolid

.changeset/fix-client-hole-retry-subscription-leak.md

@@ -0,0 +1,5 @@
+---
+"solid-js": patch
+---
+
+Fix a per-request SSR memory leak for bare `ssrSource: "client"` sources (#3657). A derived read of a client-only source (`<Show when={client().length}>`, a `dynamic()` source memo, an `<Errored>` aggregate) subscribed its retry to the shared never-settling client-hole promise; those subscriptions could never fire but were never released either, pinning each request's computation, props and data for the life of the process. The client hole is now an inert thenable rather than a native promise — `then` drops its callbacks — so no subscription site can accumulate anything on it. Rendered output is unchanged.

packages/solid/src/server/signals.ts

@@ -858,15 +858,32 @@ type NoFn<T> = T extends Function ? never : T;
 /**
  * The pending source for BARE `ssrSource: "client"` (no declared commit #0):
  * a hole the server can never fill. Reads throw a `NotReadyError` carrying
- * this promise; the `$clientHole` tag classifies the suspension as FINAL —
+ * this thenable; the `$clientHole` tag classifies the suspension as FINAL —
  * boundaries hand the position to the client (the "$$f" client-continue
- * route) instead of awaiting a settle that will never come. One shared,
- * never-settling instance: retry subscriptions attached to it (e.g.
- * `subscribePendingRetry`) are inert by design.
+ * route) instead of awaiting a settle that will never come.
+ *
+ * One shared instance, and deliberately NOT a native Promise. A never-
+ * settling Promise still records every `then`/`await` against it in its
+ * reaction list, and that list is reachable from the module scope — so each
+ * retry subscription a derived read attached (`subscribePendingRetry` from
+ * a `<Show when={client().length}>`, a `dynamic()` source memo, …) pinned
+ * that request's computation, props and data for the life of the process
+ * (#3657). This thenable's `then` drops its callbacks: subscribing to it is
+ * inert at the object, not by convention at each call site. `await` and the
+ * `Promise` combinators go through `Promise.resolve(thenable)`, which mints
+ * a fresh never-settling promise per call and holds no reference back here.
  */
-const CLIENT_HOLE: Promise<never> = /* @__PURE__ */ Object.assign(new Promise<never>(() => {}), {
-  $clientHole: true
-});
+type ClientHole = PromiseLike<never> & { $clientHole: true };
+const CLIENT_HOLE: ClientHole = /* @__PURE__ */ (() => {
+  const hole = {
+    $clientHole: true as const,
+    then: () => hole,
+    catch: () => hole,
+    finally: () => hole
+  };
+  return Object.freeze(hole) as unknown as ClientHole;
+})();
+const isClientHole = (source: unknown): boolean => source === CLIENT_HOLE;
 
 /**
  * A final (client-hole) suspension is only meaningful where a `<Loading>`
@@ -2080,7 +2097,7 @@ function serverEffect<T>(
         // response forever. Rethrow so the surrounding render (a Loading
         // discovery pass — the read throws loudly anywhere else) escalates
         // the suspension to the boundary, which hands off to the client.
-        if (source === CLIENT_HOLE) throw err;
+        if (isClientHole(source)) throw err;
         const retry = () => {
           if (comp.disposed) return;
           try {
@@ -2096,7 +2113,7 @@ function serverEffect<T>(
               // is no render on the stack to escalate to, so swallow: the
               // effect simply never fires server-side (the client runs it
               // after hydration), instead of blocking the stream forever.
-              if (next !== CLIENT_HOLE) ctx.block(next.then(retry, () => {}));
+              if (!isClientHole(next)) ctx.block(next.then(retry, () => {}));
               return;
             }
             // Out-of-band by now — route to the boundary's error handler.
@@ -2279,7 +2296,7 @@ export function createOptimisticStore<T extends object = {}>(
  */
 function createPendingProxy<T extends object>(
   state: T,
-  source: Promise<any>
+  source: PromiseLike<any>
 ): [proxy: Store<T>, markReady: (frozenState?: T) => void, markError: (error: any) => void] {
   let status: 0 | 1 | 2 = 0;
   let error: any;

packages/solid/test/server/ssr-async.spec.ts

@@ -2099,6 +2099,90 @@ describe("ssrSource server modes", () => {
       expect([...serialized.values()]).toContain("$$f");
       expect(result()).toBe("Shell");
     });
+
+    // #3657: the shared client hole is read by every request that touches a
+    // bare client source, and derived reads (a `<Show when={client().length}>`
+    // memo, an Errored aggregate's `Promise.all`) subscribe to it as their
+    // retry source. A native never-settling Promise records each of those
+    // subscriptions in a reaction list rooted in module scope — one retained
+    // computation (props, data, owner tree) per request, forever. The hole is
+    // therefore not a Promise: a frozen thenable whose `then` drops its
+    // callbacks, so no call site can accumulate anything on it.
+    test("the client hole is an inert thenable, not a native promise (#3657)", async () => {
+      const { context } = createSerializeTrackingContext();
+      sharedConfig.context = context;
+
+      let hole: any;
+      createRoot(
+        () => {
+          const read = (createMemo as any)(() => 1, { ssrSource: "client" });
+          (context as any)._loadingPhase = true;
+          try {
+            read();
+          } catch (e) {
+            hole = (e as NotReadyError).source;
+          } finally {
+            (context as any)._loadingPhase = undefined;
+          }
+        },
+        { id: "t" }
+      );
+
+      expect(hole.$clientHole).toBe(true);
+      // No reaction list exists to grow: not a promise by brand or by prototype.
+      expect(hole instanceof Promise).toBe(false);
+      expect(Object.prototype.toString.call(hole)).not.toBe("[object Promise]");
+      expect(Object.isFrozen(hole)).toBe(true);
+
+      // Every subscription shape the server takes against a pending source is
+      // dropped on the floor: direct `then` (subscribePendingRetry), `catch`
+      // (the boundary's await guard), the Promise combinators and `await`
+      // (which route through Promise.resolve and mint a fresh promise — no
+      // reference back to the hole).
+      const spy = vi.fn();
+      hole.then(spy, spy);
+      hole.catch(spy);
+      hole.finally(spy);
+      Promise.all([hole]).then(spy, spy);
+      Promise.resolve(hole).then(spy, spy);
+      expect(Promise.resolve(hole)).not.toBe(hole);
+      await tick();
+      await tick();
+      expect(spy).not.toHaveBeenCalled();
+    });
+
+    test("derived read of a client hole (Show over client().length) still hands off to $$f", () => {
+      const { context, serialized, registeredFragments } = createMockSSRContext();
+      sharedConfig.context = context;
+
+      let result: any;
+      createRoot(
+        () => {
+          result = Loading({
+            fallback: "Shell",
+            get children() {
+              const data = (createMemo as any)(() => [1, 2, 3], { ssrSource: "client" });
+              // The reporter's shape (#3657): the read is in a DERIVED memo —
+              // Show's `when` — whose update() catches the NotReady and
+              // subscribes its retry to the hole's source.
+              return ssr(["<div>", "</div>"], () =>
+                Show({
+                  get when() {
+                    return data().length;
+                  },
+                  children: "loaded"
+                })
+              ) as any;
+            }
+          });
+        },
+        { id: "t" }
+      );
+
+      expect(registeredFragments.size).toBe(0);
+      expect([...serialized.values()]).toContain("$$f");
+      expect(result()).toBe("Shell");
+    });
   });
 
   test("ssrSource 'hybrid' runs computation (same as default for Promises)", () => {