diff options
author | Bartek IwaĆczuk <biwanczuk@gmail.com> | 2021-08-23 16:15:59 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-08-23 16:15:59 +0200 |
commit | 2187c11e5d026733b4df1d675cfe5922302c8f4b (patch) | |
tree | 09df93880f4dfb1c1ad2f60e89d5e6a0df7321b9 | |
parent | 2c17045aa8fd6f01b379fbb6e5d29b9df45b9b1b (diff) |
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.
-rw-r--r-- | cli/tests/unit/http_test.ts | 24 | ||||
-rw-r--r-- | 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 f8de63383..0642a6d67 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 3f8bcb3a8..b14a0d352 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); }, }); |