Repository navigation
Stream not emitting data event after pipe+unpipe #1041
Description
Activity
you should get the error "chunk is not defined" on line 6, when fixed it should print "ok" on the stdout.
I'm sorry @micnic I forgot the chunk argument when I wrote this snipet I've modified the line.
When I exec this script nothing is logged to the console, but when I comment the pipe/unpipe line, ok is logged.var stream = require('stream'); var pass = new stream.PassThrough(); var writable = new stream.Writable(); // When the line below is commented, ok is logged //pass.pipe(writable); pass.unpipe(writable); pass.on('data', function(chunk){ console.log(chunk.toString()); }); pass.write('ok');
passgets paused and it needs to be resumed withpass.resume()before writing. I'm not sure, but this could be a bug, in node0.10it works as you expect, in0.12and io.js it needs to be resumed.You're right !
With 0.10 no need to call resume()
With 0.12 I have to call resume() to get the 'ok' logvar stream = require('stream'); var pass = new stream.PassThrough(); var writable = new stream.Writable(); pass.pipe(writable); pass.unpipe(writable); pass.resume(); pass.on('data', function(chunk){ console.log(chunk.toString()); }); pass.write('ok');
Thank you very much :)
.unpipesetsstate.flowing = false, which is the same as explicitly.pause()'ing the stream. Since it's in an explicitly paused state,.on('data')does not implicitly resume the stream. This seems like a regression. cc @iojs/streamsping @iojs/streams
@chrisdickinson This "feature" seems to have been introduced here 444bbd4
streams: Support objects other than Buffers
...
Fixed a bug with unpipe where the pipe would break because
the flowing state was not reset to false.I believe this was during the
streams2era. So in that case I think we weren't expecting to get anydataevents.We'll have to see what the implications are to streams with only a
readablelistener if we remove this.@sonewman @chrisdickinson @nodejs/streams I don't suppose there's anything new to add to this issue at this time, is there?
That's a bug indeed, tracked down in the following:
var fs = require('fs'); var src = fs.createReadStream(__filename); var dst = fs.createWriteStream('/tmp/test'); src.pipe(dst); src.unpipe(dst); src.on('data', function() { console.log("Never happens") });
As @chrisdickinson wrote above,
unpipesets the stream to "explicitly paused" state, thenon('data')does not resume it.P.S. And yes, extra
resumecall at the end makes it flow again.This sounds like it may just be a docs issue?
yes, it's a docs issue, there's another one I remember, explicitly citing docs. Maybe close this one.
at least, it looks like the code won't be fixed, so it becomes a docs issue ;)
ping @nodejs/streams
It is a doc issues, specifically we need to update https://nodejs.org/api/stream.html#stream_three_states so that it clearly states things.
- added a commit that references this issue
on Jun 5, 2017 - added a commit that references this issue
on Jun 5, 2017 - added a commit that references this issue
on Jul 17, 2017 Actually, this still breaks any
dataeven listeners added before anypipe()/unpipe(), which I believe is a serious behavior flaw introduced inReadableby 444bbd4Consider the following:
const readable; // A Readable stream const writable; // A Writable stream readable.on('data', function onData(chunk) { // This will only execute until unpipe(); }); readable.pipe(writable); readable.unpipe(writable);
Given it unexpectedly pauses the stream, even while there are
datalisteners attached, this cannot be just a docs issue.In scenarios where there are two independent algorithms consuming the stream - the first using
datalisteners and the second usingpipe- it is unfeasible for the first algorithm to monitor any following pipe/unpipe (events are on writable anyway)
For the scenario I mentioned here, I've already created a quick fix (and associated test), but I'm waiting for your thoughts before turning it into a PR.
The fix simply checks if there are any remaining
datalisteners uponcleanup()after callingunpipe(); if there are, it keeps the stream in flowing mode.
A similar argument could be made for attaching
datalisteners afterunpipe(), but that involves a slightly different fix, which requires only settingstate.flowingtonullinstead offalse(here and here) in theunpipe()function.
That would ensure the stream is in paused mode afterunpipe()and nodatalisteners, but that it resumes flow whenever there's a new consumer attached.
I'd still need to run tests for this, and it might also mean reverting the docs updateGiven it unexpectedly pauses the stream, even while there are data listeners attached, this cannot be just a docs issue.
It is a doc issue compared to the current behavior: streams currently behave in this way, and it is expected for them to keep behaving in this manner as the commit you are referencing is 5 years old. You are proposing a change to this behavior which will be semver-major and I expect it to be a non-trivial change, considering possible breakages in userland.
If you want to send a PR, please do!@mcollina many thanks for clarifying why it's been a doc issue.
However, I still believe that the current behavior should be reverted, since it breaks interoperability between the two modes of consuming a stream. I agree, this might indeed require a bit more work, and some more tests, and I don't expect it to land anytime soon.
I'll send over a PR, once I get to properly refactor some bits, and hopefully that'll start a discussion about adjusting the behavior at least, in some future version.
The following should log 'ok' to the console but nothing is logged.
Of course, commenting the line
pass.pipe(writable); pass.unpipe(writable);fix the problem.Can you explain me why?