From 0600cf16568a8fc86bc3c3bf983cecce4631e1af Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Sat, 25 May 2024 08:04:11 +0200 Subject: [PATCH] Revert "stream: revert fix cloned webstreams not being unref'd" This reverts commit 8d20b641a2ff79df804f6a0c21f2c9a0bdfe6f4a. --- lib/internal/webstreams/transfer.js | 3 +++ test/parallel/test-webstreams-clone-unref.js | 16 ++++++++++++++++ test/parallel/test-whatwg-webstreams-transfer.js | 11 +++++++++++ 3 files changed, 30 insertions(+) create mode 100644 test/parallel/test-webstreams-clone-unref.js diff --git a/lib/internal/webstreams/transfer.js b/lib/internal/webstreams/transfer.js index 4e5d94fa045f19..bc4526d13b7a3d 100644 --- a/lib/internal/webstreams/transfer.js +++ b/lib/internal/webstreams/transfer.js @@ -146,6 +146,8 @@ class CrossRealmTransformReadableSource { error); port.close(); }; + + port.unref(); } start(controller) { @@ -219,6 +221,7 @@ class CrossRealmTransformWritableSink { port.close(); }; + port.unref(); } start(controller) { diff --git a/test/parallel/test-webstreams-clone-unref.js b/test/parallel/test-webstreams-clone-unref.js new file mode 100644 index 00000000000000..88a9cebd9c3046 --- /dev/null +++ b/test/parallel/test-webstreams-clone-unref.js @@ -0,0 +1,16 @@ +'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 b5122823e46b3b..cf9b97a5ccd1c4 100644 --- a/test/parallel/test-whatwg-webstreams-transfer.js +++ b/test/parallel/test-whatwg-webstreams-transfer.js @@ -454,12 +454,23 @@ 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(); });