'close' event for spawn() isn't emitted under unknown conditions

[iamdoron]

• Version: v4.4.5
• Platform: OS X 10.11.5 (15F34) [Darwin 15.5.0 Darwin Kernel Version 15.5.0: Tue Apr 19 18:36:36 PDT 2016; root:xnu-3248.50.21~8/RELEASE_X86_64 x86_64]
• Subsystem: child_process

I wasn't able to create a code example for this issue, but is happens consistently in our integration tests. We spawn a python process (2.7.10) with binary encoding - eventually we get stderr 'end' & process 'exit' events, but no stdout 'end' or process 'close' events - it seems like it's specific to some of the tests, but not for all. I tested on v4.4.4 (and some other earlier versions) with no similar issues at all - so you might be able to pin point the issue without a proper code example.

a sketch of the code

const process = spawn(pythonPath, command, { encoding: 'binary' } );
process
.on('exit', (code) => { console.log('exit') })
.on('close', (code) => { console.log('close') }) // for some cases this event doesn't arrive

[bnoordhuis]

There is nothing in the diff between v4.4.4 and v4.4.5 that stands out. I don't think we'll be able to do much without a test case.

[iamdoron]

@bnoordhuis - I think it might be connected to gunzip, the following test case demonstrates the issue (at least on my machine)

example.sh:

#!/bin/bash
cat ./example.gz

example.gz

javascript:

const spawn = require("child_process").spawn;
const zlib = require('zlib');
const child = spawn("./example.sh")
child.on('exit', () => {
  console.log("exit");
}).on('close', () => {
  console.log("close");
})
const gunzip = zlib.createGunzip();
gunzip.on('end', () => {
  console.log("gunzip ended");
})
child.stdout.pipe(gunzip).on('data', function() {})

output on 4.4.5 (on my machine)

exit

on 4.4.4:

exit
close
gunzip ended

[addaleax]

48684af is the only zlib-related change in v4.4.5, and even though it touches the zlib close() logic, it does not seem like it could have this effect. I also can’t reproduce any change in behaviour from v4.4.4 to v4.4.5 using the above example on Linux.

There’s also my 51c0808, which touches the piping login for streams, but I wouldn’t say that that’s a likely candidate here, either.

[addaleax]

@iamdoron Does anything change from Node v4.1.2 to v4.2.0 for you?

[iamdoron]

I was able to reproduce it on ubuntu Linux 3.13.0-71-generic #114-Ubuntu SMP Tue Dec 1 02:34:22 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux

just gunzip the example.gz file and append it to a new file until you have ~96MB of unzipped file (maybe you'll need more, I guess it depends on the machine), then gzip it and try running the example again

[MylesBorins]

@addaleax I plan to take a look into this on Monday. Taking advantage of githubs new ability to assign multiple people!

Feel free to unassign

[addaleax]

Can confirm that 51c0808 is actually responsible for this… sigh.

[addaleax]

/cc @nodejs/streams I guess

[MylesBorins]

Meta data from 51c0808

Fixes: #5820
Fixes: #5257
PR-URL: #6023