Repository: denoland/deno
Pull request: fix(ext/node): emit request "close" before socket "free" in keep-alive path

## Summary
Fixes a keep-alive socket lifecycle ordering bug where `socket.emit("free")` fired before `request.on("close")` handlers had a chance to run. This caused libraries like `node-fetch` that attach per-request listeners to the socket (and clean them up on request "close") to accumulate stale listeners across reused sockets, leading to `MaxListenersExceededWarning` and eventual `Premature close` errors under concurrent load.

The fix reorders operations in `#tryReturnSocket()` to mirror Node's `emitFreeNT` ordering:

1. Emit request "close" (userland cleanup runs here)
2. Clear socket references (prevents response "close" from touching the pooled socket)
3. Emit socket "free" (agent reuses socket — cleanup already happened)

Fixes #32780

## Test plan
  - [x] New spec test: `http_keepalive_close_before_free` — verifies per-request
    socket listeners are cleaned up before socket reuse across 15 sequential
    keep-alive requests
  - [x] Existing `http_agent_keepalive` tests pass
  - [x] Existing `http_agent_keepalive_stale_*` tests pass
  - [x] Verify that original repro (see #32780) is fixed 

Changed files:
filename: ext/node/polyfills/http.ts
patch: @@ -1305,13 +1305,24 @@ export class IncomingMessageForClient extends NodeReadable {
         req._socketErrorListener = null;
       }
 
-      socket.emit("free");
-
-      // Clear references so old request/response don't destroy the pooled socket.
+      // Mirror Node's emitFreeNT ordering: emit request "close"
+      // before socket "free" so userland cleanup (e.g. node-fetch
+      // removing per-request socket listeners) runs before the
+      // agent reuses the socket.
       if (req) {
+        req.destroyed = true;
+        req._closed = true;
+        req.emit("close");
         req.socket = null;
       }
+
+      // Clear response's socket reference so that when push(null)
+      // triggers the response "close" listener, it won't emit
+      // "close" on the pooled socket. This must be after req.emit("close")
+      // so cleanup handlers can still access the socket via res.socket.
       this.socket = null;
+
+      socket.emit("free");
     } catch (_e) {
       // Socket reuse is best-effort.
     }

filename: tests/unit_node/http_test.ts
patch: @@ -2317,6 +2317,82 @@ Deno.test("[node/http] ServerResponse.writeEarlyHints", async () => {
   await promise;
 });
 
+// https://github.com/denoland/deno/issues/32780
+Deno.test({
+  name: "[node/http] keep-alive request close fires before socket free",
+  sanitizeResources: false,
+  async fn() {
+    const { promise, resolve: done } = Promise.withResolvers<void>();
+
+    const agent = new http.Agent({
+      keepAlive: true,
+      maxSockets: 1,
+      maxFreeSockets: 1,
+    });
+
+    const server = http.createServer((_req, res) => {
+      res.end("ok");
+    });
+
+    // Capture the shared socket so we can check its listener count
+    // even after res.socket is nulled during the keep-alive handoff.
+    let sharedSocket: Socket | null = null;
+
+    function makeRequest(path: string): Promise<void> {
+      return new Promise((resolve, reject) => {
+        const req = http.get(
+          {
+            host: "127.0.0.1",
+            port: (server.address() as { port: number }).port,
+            agent,
+            path,
+          },
+          (res) => {
+            const sock = res.socket!;
+            if (!sharedSocket) sharedSocket = sock;
+
+            // Attach a per-request listener on the socket, mimicking what
+            // node-fetch and similar libraries do.
+            const onData = () => {};
+            sock.on("data", onData);
+
+            // Clean it up on request close — this must fire BEFORE the
+            // agent reuses the socket, otherwise the listener leaks.
+            req.on("close", () => {
+              sock.removeListener("data", onData);
+            });
+
+            res.on("data", () => {});
+            res.on("end", () => resolve());
+          },
+        );
+        req.on("error", reject);
+      });
+    }
+
+    server.listen(0, async () => {
+      for (let i = 0; i < 15; i++) {
+        await makeRequest(`/req-${i}`);
+        await new Promise((resolve) => setTimeout(resolve, 5));
+      }
+
+      // Without the fix, req "close" never fires in the keep-alive path,
+      // so all 15 "data" listeners accumulate on the socket.
+      // With the fix, each listener is cleaned up before socket reuse.
+      const leakedListeners = sharedSocket!.listenerCount("data");
+      assert(
+        leakedListeners === 0,
+        `Expected 0 "data" listeners on the socket, but found ${leakedListeners}`,
+      );
+
+      agent.destroy();
+      server.close(() => done());
+    });
+
+    await promise;
+  },
+});
+
 // https://github.com/denoland/deno/issues/32311
 Deno.test("[node/http] upgrade request can be rejected with non-101 status", async () => {
   const { promise, resolve } = Promise.withResolvers<void>();