Skip to content

Commit e443953

Browse files
jasnellRafaelGSS
authored andcommitted
stream: fix cloned webstreams not being unref'd
When cloning a `ReadableStream` and `WritableStream`, both use an internal `MessageChannel` to communicate with the original stream. Those, however, previously were not unref'd which would lead to the process not exiting if the stream was not fully consumed. Fixes: #44985 PR-URL: #51255 Reviewed-By: Matthew Aitken <maitken033380023@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
1 parent cd61fce commit e443953

File tree

5 files changed

+34
-1
lines changed

5 files changed

+34
-1
lines changed

lib/internal/webstreams/readablestream.js

+2
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,8 @@ class ReadableStream {
609609

610610
[kTransferList]() {
611611
const { port1, port2 } = new MessageChannel();
612+
port1.unref();
613+
port2.unref();
612614
this[kState].transfer.port1 = port1;
613615
this[kState].transfer.port2 = port2;
614616
return [ port2 ];

lib/internal/webstreams/transfer.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,8 @@ class CrossRealmTransformReadableSource {
143143
error);
144144
port.close();
145145
};
146+
147+
port.unref();
146148
}
147149

148150
start(controller) {
@@ -210,7 +212,7 @@ class CrossRealmTransformWritableSink {
210212
error);
211213
port.close();
212214
};
213-
215+
port.unref();
214216
}
215217

216218
start(controller) {

lib/internal/webstreams/writablestream.js

+2
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,8 @@ class WritableStream {
304304

305305
[kTransferList]() {
306306
const { port1, port2 } = new MessageChannel();
307+
port1.unref();
308+
port2.unref();
307309
this[kState].transfer.port1 = port1;
308310
this[kState].transfer.port2 = port2;
309311
return [ port2 ];
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
'use strict';
2+
3+
require('../common');
4+
const { ok } = require('node:assert');
5+
6+
// This test verifies that cloned ReadableStream and WritableStream instances
7+
// do not keep the process alive. The test fails if it timesout (it should just
8+
// exit immediately)
9+
10+
const rs1 = new ReadableStream();
11+
const ws1 = new WritableStream();
12+
13+
const [rs2, ws2] = structuredClone([rs1, ws1], { transfer: [rs1, ws1] });
14+
15+
ok(rs2 instanceof ReadableStream);
16+
ok(ws2 instanceof WritableStream);

test/parallel/test-whatwg-webstreams-transfer.js

+11
Original file line numberDiff line numberDiff line change
@@ -464,12 +464,23 @@ const theData = 'hello';
464464
tracker.verify();
465465
});
466466
467+
// We create an interval to keep the event loop alive while
468+
// we wait for the stream read to complete. The reason this is needed is because there's
469+
// otherwise nothing to keep the worker thread event loop alive long enough to actually
470+
// complete the read from the stream. Under the covers the ReadableStream uses an
471+
// unref'd MessagePort to communicate with the main thread. Because the MessagePort
472+
// is unref'd, it's existence would not keep the thread alive on its own. There was previously
473+
// a bug where this MessagePort was ref'd which would block the thread and main thread
474+
// from terminating at all unless the stream was consumed/closed.
475+
const i = setInterval(() => {}, 1000);
476+
467477
parentPort.onmessage = tracker.calls(({ data }) => {
468478
assert(isReadableStream(data));
469479
const reader = data.getReader();
470480
reader.read().then(tracker.calls((result) => {
471481
assert(!result.done);
472482
assert(result.value instanceof Uint8Array);
483+
clearInterval(i);
473484
}));
474485
parentPort.close();
475486
});

0 commit comments

Comments
 (0)