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

close versus end event #27

Closed
jeromew opened this issue Aug 15, 2014 · 5 comments
Closed

close versus end event #27

jeromew opened this issue Aug 15, 2014 · 5 comments

Comments

@jeromew
Copy link

jeromew commented Aug 15, 2014

I have an issue with raw-body on the way it handles the 'close' event.

currently, if I

1. receive 'data'
2. receive 'close'
3. receive 'end'

then the buffer is empty because 'close' triggers a cleanup.

According to node.js documentation regarding the 'close' event, it is "Emitted when the underlying resource (for example, the backing file descriptor) has been closed. Not all streams will emit this."

to me, this means that the "underlying" ressource has been closed (a file descriptor for example). it does not mean that 'end' has already been called if there is still data in the internal stream buffer.

can we remove the handling of close altogether ?

@jonathanong
Copy link
Contributor

no. this is to avoid leaks. close should be emitted last. are you creating your own streams or something?

@dougwilson
Copy link
Member

idk. this does sound like an issue, because technically the node.js core docs are clear in that close could occur before or after end; it's not strictly guaranteed. some stuff should really just get refactored so received = buffer = null is not necessary. this module doesn't work will with non-IncomingMessage streams, though, which always emit close after end, which is why it was never brought up before now.

@jonathanong
Copy link
Contributor

yeah i guess i disagree with node's official definition. my definition of close would be:

when the stream and any of its underlying resources have been closed, and no more events will be emitted, nor will any further computation occur.

which is pretty much how most streams in node function. but if there's a stream you can't fix, show it to us!

@dougwilson
Copy link
Member

yea. it is probably an issue here, but right now it's not a super priority for us to fix ourselves :)

@jeromew
Copy link
Author

jeromew commented Aug 15, 2014

I meet this "close before end" problem with spawning processes ; it seems to depend on how the binary handles the stdin/out, and on the size of the output. I'll keep looking.

Regarding the definition, it makes sense to me to receive an event when the underlying ressource is closed, which hopefully happens before all the data is piped downstream.

I can Imagine reading a file (<64K so everything will fit into the watermark for files) and then send it to a very slow consumer (a serial cable to configure a device for example) ; I don't want the file to be kept open if everything is already buffered) ; but that is more a "concept idea" than the bug I am trying to fix.

I'll close this for now thanks for the feedback.

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

No branches or pull requests

3 participants