-
Notifications
You must be signed in to change notification settings - Fork 29.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bug: using structuredClone with ReadableStream prevents process from exiting #44985
Comments
A workaround is to |
Ref: nodejs/undici#1700 |
It looks like 2 ports aren't unref'd [
MessagePort [EventTarget] {
active: true,
refed: true,
[Symbol(kEvents)]: SafeMap(4) [Map] {
'newListener' => [Object],
'removeListener' => [Object],
'message' => [Object],
'messageerror' => [Object]
},
[Symbol(events.maxEventTargetListeners)]: 10,
[Symbol(events.maxEventTargetListenersWarned)]: false,
[Symbol(kNewListener)]: [Function (anonymous)],
[Symbol(kRemoveListener)]: [Function (anonymous)],
[Symbol(nodejs.internal.kCurrentlyReceivingPorts)]: undefined,
[Symbol(khandlers)]: SafeMap(2) [Map] {
'message' => [Function],
'messageerror' => [Function]
}
},
MessagePort [EventTarget] {
active: true,
refed: true,
[Symbol(kEvents)]: SafeMap(4) [Map] {
'newListener' => [Object],
'removeListener' => [Object],
'message' => [Object],
'messageerror' => [Object]
},
[Symbol(events.maxEventTargetListeners)]: 10,
[Symbol(events.maxEventTargetListenersWarned)]: false,
[Symbol(kNewListener)]: [Function (anonymous)],
[Symbol(kRemoveListener)]: [Function (anonymous)],
[Symbol(nodejs.internal.kCurrentlyReceivingPorts)]: undefined,
[Symbol(khandlers)]: SafeMap(2) [Map] {
'message' => [Function],
'messageerror' => [Function]
}
}
] which from my best guess are these: node/lib/internal/webstreams/readablestream.js Lines 579 to 580 in 2672219
|
@ronag const { kTransfer } = require("internal/worker/js_transferable");
const { readableStreamPipeTo } = require("internal/webstreams/readablestream");
const { setPromiseHandled, kState } = require("internal/webstreams/util");
const {
CrossRealmTransformWritableSink,
} = require("internal/webstreams/transfer");
function newCrossRealmWritableSink(readable, port) {
const source = new CrossRealmTransformWritableSink(port);
const writable = new WritableStream(source);
const promise = readableStreamPipeTo(readable, writable, false, false, false);
setPromiseHandled(promise);
return {
writable,
source,
promise,
};
}
const registry = new FinalizationRegistry(({ source }) => {
source.close();
});
ReadableStream.prototype[kTransfer] = function () {
if (this.locked) {
this[kState].transfer.port1?.close();
this[kState].transfer.port1 = undefined;
this[kState].transfer.port2 = undefined;
throw new DOMException(
"Cannot transfer a locked ReadableStream",
"DataCloneError"
);
}
const { port1, port2 } = this[kState].transfer;
this[kState].transfer.port2 = undefined;
const { writable, promise, source } = newCrossRealmWritableSink(this, port1);
this[kState].transfer.writable = writable;
this[kState].transfer.promise = promise;
registry.register(port2, { source });
return {
data: { port: port2 },
deserializeInfo:
"internal/webstreams/readablestream:TransferredReadableStream",
};
};
const stack = [
new Uint8Array([65]),
new Uint8Array([65]),
new Uint8Array([65]),
];
const rs = new ReadableStream({
pull(controller) {
const data = stack.shift();
if (data) {
controller.enqueue(data);
} else {
controller.close();
}
},
});
const cloned = structuredClone(rs, { transfer: [rs] }); |
@ronag |
Sorry. Webstreams are beyond my expertise and frankly my interest... |
I apologize for the inconvenience that I have caused you. |
No worries. No inconvenience at all. 🤗 |
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: nodejs#44985
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: nodejs#44985
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>
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>
Reopening after #51491 |
The Node.js bug preventing it from being used (nodejs/node#44985) has been fixed in all active branches except for 18.x. Support for this branch will be dropped and the minimum Node.js requirement will be bumped.
Version
v18.10.0
Platform
Microsoft Windows NT 10.0.19043.0 x64
Subsystem
No response
What steps will reproduce the bug?
After this the process will indefinitely stay open.
How often does it reproduce? Is there a required condition?
always
What is the expected behavior?
the process should exit
What do you see instead?
the process stays open indefinitely
Additional information
Exporting readableStreamTee would also solve my use case.
node/lib/internal/webstreams/readablestream.js
Line 1443 in 214354f
The text was updated successfully, but these errors were encountered: