From 8e8245196af17b32ae417f3d3eb72ed6527c6872 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 16 Jan 2024 16:45:52 +0100 Subject: [PATCH] Revert "stream: fix cloned webstreams not being unref'd" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 4d3923aabaa6d57dc9dc3c554434fa69410c3d55. PR-URL: https://github.com/nodejs/node/pull/51491 Reviewed-By: Vinícius Lourenço Claro Cardoso Reviewed-By: Marco Ippolito Reviewed-By: Matthew Aitken Reviewed-By: Rafael Gonzaga Reviewed-By: Moshe Atlow Reviewed-By: Franziska Hinkelmann Reviewed-By: Benjamin Gruenbaum --- lib/internal/webstreams/readablestream.js | 2 -- lib/internal/webstreams/transfer.js | 4 +--- lib/internal/webstreams/writablestream.js | 2 -- test/parallel/test-webstreams-clone-unref.js | 16 ---------------- test/parallel/test-whatwg-webstreams-transfer.js | 11 ----------- 5 files changed, 1 insertion(+), 34 deletions(-) delete mode 100644 test/parallel/test-webstreams-clone-unref.js diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js index 5103591312e479..62dfd8a288ab4c 100644 --- a/lib/internal/webstreams/readablestream.js +++ b/lib/internal/webstreams/readablestream.js @@ -598,8 +598,6 @@ class ReadableStream { [kTransferList]() { const { port1, port2 } = new MessageChannel(); - port1.unref(); - port2.unref(); this[kState].transfer.port1 = port1; this[kState].transfer.port2 = port2; return [ port2 ]; diff --git a/lib/internal/webstreams/transfer.js b/lib/internal/webstreams/transfer.js index c4cb4077f88403..136b0d81a99464 100644 --- a/lib/internal/webstreams/transfer.js +++ b/lib/internal/webstreams/transfer.js @@ -143,8 +143,6 @@ class CrossRealmTransformReadableSource { error); port.close(); }; - - port.unref(); } start(controller) { @@ -212,7 +210,7 @@ class CrossRealmTransformWritableSink { error); port.close(); }; - port.unref(); + } start(controller) { diff --git a/lib/internal/webstreams/writablestream.js b/lib/internal/webstreams/writablestream.js index 3bc77fc6fb7067..eea99f321d50c6 100644 --- a/lib/internal/webstreams/writablestream.js +++ b/lib/internal/webstreams/writablestream.js @@ -274,8 +274,6 @@ class WritableStream { [kTransferList]() { const { port1, port2 } = new MessageChannel(); - port1.unref(); - port2.unref(); this[kState].transfer.port1 = port1; this[kState].transfer.port2 = port2; return [ port2 ]; diff --git a/test/parallel/test-webstreams-clone-unref.js b/test/parallel/test-webstreams-clone-unref.js deleted file mode 100644 index 88a9cebd9c3046..00000000000000 --- a/test/parallel/test-webstreams-clone-unref.js +++ /dev/null @@ -1,16 +0,0 @@ -'use strict'; - -require('../common'); -const { ok } = require('node:assert'); - -// This test verifies that cloned ReadableStream and WritableStream instances -// do not keep the process alive. The test fails if it timesout (it should just -// exit immediately) - -const rs1 = new ReadableStream(); -const ws1 = new WritableStream(); - -const [rs2, ws2] = structuredClone([rs1, ws1], { transfer: [rs1, ws1] }); - -ok(rs2 instanceof ReadableStream); -ok(ws2 instanceof WritableStream); diff --git a/test/parallel/test-whatwg-webstreams-transfer.js b/test/parallel/test-whatwg-webstreams-transfer.js index 7be01c339652c0..01cfaa02ad075e 100644 --- a/test/parallel/test-whatwg-webstreams-transfer.js +++ b/test/parallel/test-whatwg-webstreams-transfer.js @@ -464,23 +464,12 @@ const theData = 'hello'; tracker.verify(); }); - // We create an interval to keep the event loop alive while - // we wait for the stream read to complete. The reason this is needed is because there's - // otherwise nothing to keep the worker thread event loop alive long enough to actually - // complete the read from the stream. Under the covers the ReadableStream uses an - // unref'd MessagePort to communicate with the main thread. Because the MessagePort - // is unref'd, it's existence would not keep the thread alive on its own. There was previously - // a bug where this MessagePort was ref'd which would block the thread and main thread - // from terminating at all unless the stream was consumed/closed. - const i = setInterval(() => {}, 1000); - parentPort.onmessage = tracker.calls(({ data }) => { assert(isReadableStream(data)); const reader = data.getReader(); reader.read().then(tracker.calls((result) => { assert(!result.done); assert(result.value instanceof Uint8Array); - clearInterval(i); })); parentPort.close(); });