-
Notifications
You must be signed in to change notification settings - Fork 31.1k
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
stream: 'prefinish' timing and 'finish' after 'close' #31401
Labels
stream
Issues and PRs related to the stream subsystem.
Comments
One of these will fail while the other won't: {
const write = new Writable({
write(chunk, enc, cb) { cb(); },
final(cb) { cb(); }
});
write.on('close', common.mustCall(() => {
write.on('finish', common.mustNotCall());
}));
write.on('prefinish', common.mustCall(() => {
write.destroy();
}));
write.end();
}
{
const write = new Writable({
write(chunk, enc, cb) { cb(); }
});
write.on('close', common.mustCall(() => {
write.on('finish', common.mustNotCall()); // fails
}));
write.on('prefinish', common.mustCall(() => {
write.destroy();
}));
write.end();
} |
4 tasks
ronag
added a commit
to nxtedition/node
that referenced
this issue
Apr 15, 2020
This PR fixes a few different things: The timing of 'prefinish' depends on whether or not _final is defined. In on case the event is emitted synchronously with end() and otherwise asynchronously. _final is currently unecessarily called asynchronously which forces implementors to use 'prefinish' as a hack to emulate synchronous behaviour. Furthermore, this hack is subtly broken due to the above issue. The stream should not finish if errored or destroyed synchronously during the prefinish stage. Refs: nodejs#31401 Refs: nodejs#32763 (comment)
ronag
added a commit
that referenced
this issue
Apr 22, 2020
This PR fixes a few different things: The timing of 'prefinish' depends on whether or not _final is defined. In on case the event is emitted synchronously with end() and otherwise asynchronously. _final is currently unecessarily called asynchronously which forces implementors to use 'prefinish' as a hack to emulate synchronous behaviour. Furthermore, this hack is subtly broken due to the above issue. Refs: #31401 Refs: #32763 (comment) PR-URL: #32780 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Rich Trott <[email protected]>
what's the status on this? |
I think this is fixed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
'finish'
can be emitted after'close'
ifdestroy()
is called inside a'prefinish'
handler.'prefinish'
has different timing (sync vs async) depending on whetherWritable
implements_final
which can cause problems such as above to occur or not occur. (stream: simplify Transform stream implementation #32763)I assume'prefinish'
is mostly used internally so it's probably not a huge issue.The text was updated successfully, but these errors were encountered: