From 2187c11e5d026733b4df1d675cfe5922302c8f4b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= <biwanczuk@gmail.com>
Date: Mon, 23 Aug 2021 16:15:59 +0200
Subject: [PATCH] fix(ext/http): resource leak on HttpConn.close() (#11805)

This commit adds tracking of resources that are related
to "HttpConn" so they can be closed automatically
when closing the connection.
---
 cli/tests/unit/http_test.ts | 24 ++++++++++++++++++++++++
 ext/http/01_http.js         | 28 ++++++++++++++++++++++++++--
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/cli/tests/unit/http_test.ts b/cli/tests/unit/http_test.ts
index f8de633830..0642a6d673 100644
--- a/cli/tests/unit/http_test.ts
+++ b/cli/tests/unit/http_test.ts
@@ -815,3 +815,27 @@ unitTest(
     }
   },
 );
+
+// https://github.com/denoland/deno/issues/11743
+unitTest(
+  { perms: { net: true } },
+  async function httpServerDoesntLeakResources() {
+    const listener = Deno.listen({ port: 4505 });
+    const [conn, clientConn] = await Promise.all([
+      listener.accept(),
+      Deno.connect({ port: 4505 }),
+    ]);
+    const httpConn = Deno.serveHttp(conn);
+
+    await Promise.all([
+      httpConn.nextRequest(),
+      clientConn.write(new TextEncoder().encode(
+        `GET / HTTP/1.1\r\nHost: 127.0.0.1:4505\r\n\r\n`,
+      )),
+    ]);
+
+    httpConn.close();
+    listener.close();
+    clientConn.close();
+  },
+);
diff --git a/ext/http/01_http.js b/ext/http/01_http.js
index 3f8bcb3a8b..b14a0d3528 100644
--- a/ext/http/01_http.js
+++ b/ext/http/01_http.js
@@ -24,6 +24,10 @@
     ArrayPrototypePush,
     ArrayPrototypeSome,
     Promise,
+    Set,
+    SetPrototypeAdd,
+    SetPrototypeDelete,
+    SetPrototypeValues,
     StringPrototypeIncludes,
     StringPrototypeToLowerCase,
     StringPrototypeSplit,
@@ -38,6 +42,11 @@
 
   class HttpConn {
     #rid = 0;
+    // This set holds resource ids of resources
+    // that were created during lifecycle of this request.
+    // When the connection is closed these resources should be closed
+    // as well.
+    managedResources = new Set();
 
     constructor(rid) {
       this.#rid = rid;
@@ -85,7 +94,8 @@
       /** @type {ReadableStream<Uint8Array> | undefined} */
       let body = null;
       if (typeof requestRid === "number") {
-        body = createRequestBodyStream(requestRid);
+        SetPrototypeAdd(this.managedResources, requestRid);
+        body = createRequestBodyStream(this, requestRid);
       }
 
       const innerRequest = newInnerRequest(
@@ -97,6 +107,7 @@
       const signal = abortSignal.newSignal();
       const request = fromInnerRequest(innerRequest, signal, "immutable");
 
+      SetPrototypeAdd(this.managedResources, responseSenderRid);
       const respondWith = createRespondWith(
         this,
         responseSenderRid,
@@ -108,6 +119,13 @@
 
     /** @returns {void} */
     close() {
+      for (const rid of SetPrototypeValues(this.managedResources)) {
+        try {
+          core.close(rid);
+        } catch (_e) {
+          // pass, might have already been closed
+        }
+      }
       core.close(this.#rid);
     }
 
@@ -178,6 +196,7 @@
         respBody = new Uint8Array(0);
       }
 
+      SetPrototypeDelete(httpConn.managedResources, responseSenderRid);
       let responseBodyRid;
       try {
         responseBodyRid = await core.opAsync("op_http_response", [
@@ -200,6 +219,7 @@
       // If `respond` returns a responseBodyRid, we should stream the body
       // to that resource.
       if (responseBodyRid !== null) {
+        SetPrototypeAdd(httpConn.managedResources, responseBodyRid);
         try {
           if (respBody === null || !(respBody instanceof ReadableStream)) {
             throw new TypeError("Unreachable");
@@ -231,6 +251,7 @@
         } finally {
           // Once all chunks are sent, and the request body is closed, we can
           // close the response body.
+          SetPrototypeDelete(httpConn.managedResources, responseBodyRid);
           try {
             await core.opAsync("op_http_response_close", responseBodyRid);
           } catch { /* pass */ }
@@ -280,7 +301,7 @@
     };
   }
 
-  function createRequestBodyStream(requestRid) {
+  function createRequestBodyStream(httpConn, requestRid) {
     return new ReadableStream({
       type: "bytes",
       async pull(controller) {
@@ -298,6 +319,7 @@
           } else {
             // We have reached the end of the body, so we close the stream.
             controller.close();
+            SetPrototypeDelete(httpConn.managedResources, requestRid);
             core.close(requestRid);
           }
         } catch (err) {
@@ -305,10 +327,12 @@
           // error.
           controller.error(err);
           controller.close();
+          SetPrototypeDelete(httpConn.managedResources, requestRid);
           core.close(requestRid);
         }
       },
       cancel() {
+        SetPrototypeDelete(httpConn.managedResources, requestRid);
         core.close(requestRid);
       },
     });