Piping from stdin is broken

[addaleax]

• Version: master (ace1009 and upwards, everything before is fine)
• Platform: Linux 4.2.0-34-generic Gitter chat room? #39-Ubuntu SMP Thu Mar 10 22:13:01 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
• Subsystem: stream

Since ace1009 (#5776), stdin is truncated to the first 64kb when piped to some other stream.

Before:

$ head -c 256000 /dev/zero | ./node -e 'process.stdin.pipe(process.stdout)' | wc
      0       0  256000

After:

$ head -c 256000 /dev/zero | ./node -e 'process.stdin.pipe(process.stdout)' | wc
      0       0   65536

This also goes for piping to bl(), and the end event on process.stdin seems to be fired only when the input is smaller than 16384 bytes.

/cc @orangemocha

[addaleax]

Btw, apparently this only happens with pipes, i.e.

$ head -c 256000 /dev/zero > data
$ ./node -e 'process.stdin.pipe(process.stdout)' < data | wc
      0       0  256000

works just fine.

[Fishrock123]

cc @nodejs/streams

[Fishrock123]

Why it only happens with pipes probably has something to do with what I just posted in #5916 (comment):

<-ing a file into stdin actually results in a fs.ReadStream, rather an a tty.ReadStream, and as such does not inherit from net.Socket, unlike the other possible stdin options:

node/lib/internal/process/stdio.js

Lines 54 to 57 in 293fd04

	case 'FILE':
	var fs = require('fs');
	stdin = new fs.ReadStream(null, { fd: fd, autoClose: false });
	break;

[addaleax]

@Fishrock123 Makes sense then, thanks for the info!

[orangemocha]

I will definitely follow up and investigate this, but it might take me a while to fix it.. streams is delicate code, and I am travelling to the VM summit in SF next week. I might end up reverting #5776 while I figure it out, as it seems that this issue is more severe.

Of course, if anyone wants to investigate further please be welcome! As a first step I'd like to see if reverting 4611389 (but not ace1009) makes this problem go away or not.

[addaleax]

@evanlucas Thanks for taking care of this for now. Would it be a good idea if I tried to write a regression test for this?

[evanlucas]

@addaleax That would definitely be appreciated!

[addaleax]

@evanlucas Done: #5949

[MylesBorins]

@orangemocha I've tried reapplying ace1009 to master and running CI to see if the suite passes on windows https://ci.nodejs.org/job/node-test-commit/2887/

edit: no bueno

[orangemocha]

Thanks @thealphanerd . I am investigating again, teaming up with @joaocgreis and @bzoz, and we have also confirmed that emitting 'pause' on nextTick alone causes this issue.

This is unfortunate, because in some of my testing to fix #5384 I could see 'pause' and 'resume' events being emitted out of order. I have a bad feeling that there might be another set of hidden bugs in Node because of that, but that they are not getting caught by our unit tests.