Skip to content
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.finished readonly doesn't fire if writable side is open #32965

Closed
mafintosh opened this issue Apr 21, 2020 · 2 comments
Closed

stream.finished readonly doesn't fire if writable side is open #32965

mafintosh opened this issue Apr 21, 2020 · 2 comments
Assignees

Comments

@mafintosh
Copy link
Member

  • Version: master (b53cae3, 14-pre)
  • Platform: mac
  • Subsystem: stream

stream.finished(duplex, { readable: true, writable: false }, cb) doesn't fire if writable side is finalising.

const stream = require('stream')

const dup = new stream.Duplex({
  final (cb) { }, // never close writable side for test purpose
  read () {
    this.push(null)
  }
})

dup.on('end', function () {
  console.log('end fired')
})

stream.finished(dup, { readable: true, writable: false }, function () {
  // doesn't fire in master
  console.log('not readable')
})

dup.end()
dup.resume()
@mafintosh
Copy link
Member Author

cc @mcollina @ronag

@ronag ronag self-assigned this Apr 21, 2020
@ronag
Copy link
Member

ronag commented Apr 21, 2020

This is because of willEmitClose. We incorrectly assume that a Duplex which we only care about the readable side of, will emit close.

ronag added a commit to nxtedition/node that referenced this issue Apr 21, 2020
If passed a Duplex where readable or writable has been
explicitly disabled then don't assume 'close' will be
emitted.

Fixes: nodejs#32965
@ronag ronag closed this as completed in 4eb1701 Apr 23, 2020
BethGriggs pushed a commit that referenced this issue Apr 27, 2020
If passed a Duplex where readable or writable has been
explicitly disabled then don't assume 'close' will be
emitted.

Fixes: #32965

PR-URL: #32967
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Mathias Buus <mathiasbuus@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
BethGriggs pushed a commit that referenced this issue Apr 28, 2020
If passed a Duplex where readable or writable has been
explicitly disabled then don't assume 'close' will be
emitted.

Fixes: #32965

PR-URL: #32967
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Mathias Buus <mathiasbuus@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants