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

"write after end" error when stdout was end()-ed #9403

Closed
Fishrock123 opened this issue Nov 1, 2016 · 7 comments
Closed

"write after end" error when stdout was end()-ed #9403

Fishrock123 opened this issue Nov 1, 2016 · 7 comments
Labels
confirmed-bug Issues with confirmed bugs. process Issues and PRs related to the process subsystem. stream Issues and PRs related to the stream subsystem.

Comments

@Fishrock123
Copy link
Contributor

  • Version: master, presumably all recent node major versions
  • Platform: OS X 10.10.5
  • Subsystem: process/console

stdout/stderr unexpectedly throw a write after end error when end() has been called on them, even though end() throws it's own error in destroy()/destroySoon() overriding the usual destroy code which is in net.Socket. Since it overrides the destroy code, I don't think it should ever actually destroy the stream, which the error seems to indicate, but it appears to destroy it nonetheless.

Try in the REPL:

process.stderr.end('foo')
// throws error, REPL catches it

console.error('hello?')
// Unexpectedly throws "write after end"

cc @nodejs/streams

@Fishrock123 Fishrock123 added the process Issues and PRs related to the process subsystem. label Nov 1, 2016
@addaleax addaleax added the stream Issues and PRs related to the stream subsystem. label Nov 1, 2016
@addaleax
Copy link
Member

addaleax commented Nov 1, 2016

I think this is because .end() does end stderr from the streams perspective. So maybe .end should be overwritten, too?

@Fishrock123
Copy link
Contributor Author

Fishrock123 commented Nov 1, 2016

@addaleax Just noticed that, I think re-setting the .ended property to false might fix it. Looking into it more.

@mcollina
Copy link
Member

mcollina commented Nov 1, 2016

IMHO it is legit behavior. Maybe the error should be better/etc, but if end() is called on stderr, stderr needs to be closed.

@Fishrock123
Copy link
Contributor Author

@mcollina the error is legit but not particularly useful, see #831 for that discussion.

@mcollina
Copy link
Member

mcollina commented Nov 1, 2016

How is this different from #831?
The error on console should probably be "stderr/stdout closed".

I am missing what are you proposing regarding destroy/destroySoon.

@Fishrock123
Copy link
Contributor Author

@mcollina That is about making console safe in general, this is a bug because we actually skip closing the socket already but it thinks it is closed after.

@Fishrock123 Fishrock123 added the confirmed-bug Issues with confirmed bugs. label Nov 1, 2016
Fishrock123 added a commit to Fishrock123/node that referenced this issue Nov 4, 2016
This is a step in making `console` safe, swallows EPIPE and other
errors by registering an empty 'error' event listener.

Also fixes writes to stdout/stderr not working after `end()` is called,
even though we already override `destroy()` so the stream is never
actually closed.

Refs: nodejs#831
Fixes: nodejs#947
Fixes: nodejs#9403
@mcollina
Copy link
Member

This was fixed in #9470

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. process Issues and PRs related to the process subsystem. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants