From 2929313652a9d7fb3a26ad7f754deaeb03435e10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 29 Aug 2023 14:13:58 +0200 Subject: [PATCH] fix(node/http): don't leak resources on destroyed request (#20040) Closes https://github.com/denoland/deno/issues/19828 --- cli/tests/unit_node/http_test.ts | 34 ++++++++++++++++++++++++++++++++ ext/node/polyfills/http.ts | 7 +++++++ 2 files changed, 41 insertions(+) diff --git a/cli/tests/unit_node/http_test.ts b/cli/tests/unit_node/http_test.ts index 9feee02728..825815ae64 100644 --- a/cli/tests/unit_node/http_test.ts +++ b/cli/tests/unit_node/http_test.ts @@ -777,3 +777,37 @@ Deno.test("[node/http] server emits error if addr in use", async () => { `Wrong error: ${err.message}`, ); }); + +Deno.test( + "[node/http] client destroy doesn't leak", + { permissions: { net: true } }, + async () => { + const ac = new AbortController(); + let timerId; + + const server = Deno.serve( + { port: 5929, signal: ac.signal }, + async (_req) => { + await new Promise((resolve) => { + timerId = setTimeout(resolve, 5000); + }); + return new Response("hello"); + }, + ); + const promise = deferred(); + + const request = http.request("http://localhost:5929/"); + request.on("error", promise.reject); + request.on("close", () => {}); + request.end(); + setTimeout(() => { + request.destroy(new Error()); + promise.resolve(); + }, 100); + + await promise; + clearTimeout(timerId); + ac.abort(); + await server.finished; + }, +); diff --git a/ext/node/polyfills/http.ts b/ext/node/polyfills/http.ts index 1103e93c31..ae670d5f03 100644 --- a/ext/node/polyfills/http.ts +++ b/ext/node/polyfills/http.ts @@ -787,6 +787,13 @@ class ClientRequest extends OutgoingMessage { } this.destroyed = true; + const rid = this._client?.rid; + if (rid) { + core.tryClose(rid); + } + if (this._req.cancelHandleRid !== null) { + core.tryClose(this._req.cancelHandleRid); + } // If we're aborting, we don't care about any more response data. if (this.res) { this.res._dump();