nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.42k stars 29.52k forks source link

win, cluster: no callback called after write #19452

Open bzoz opened 6 years ago

bzoz commented 6 years ago

On Windows, after calling process.disconnect callbacks for previous process.send calls are not called. Example:

'use strict';
const cluster = require('cluster');

if (cluster.isMaster) {
  cluster.fork().on('message', (msg) => {
    console.log(msg.msg);
  });
} else {
  process.send({msg: 'hello'}, () => {
    console.log('cb!');
  });
  process.disconnect();
}

On Windows this will print:

hello

whereas on Linux it will print

hello
cb!

This was reported in https://github.com/libuv/libuv/issues/1729, but it looks like some issue with cluster module. Callbacks are called on both platforms in the following code, which uses bare IPC pipe:

const cp = require('child_process');

if (process.argv[2] === 'child') {
  process.send({msg: 'hello'}, () => {
    console.log('cb!');
  });
  process.disconnect();
} else {
  const child = cp.spawn(process.argv0, [__filename, 'child'], {
    stdio: ['inherit', 'inherit', 'inherit', 'ipc']
  });
  child.on('message', (msg) => {
    console.log(msg.msg);
  })
}

On Windows this will print:

hello
cb!

and on Linux:

cb!
hello

/cc @nodejs/platform-windows @nodejs/cluster

oyyd commented 6 years ago

Try to investigate why this happens these days:

https://github.com/nodejs/node/blob/af7164ebccd21d9fc5b0782e0427257f7637a4db/lib/internal/cluster/child.js#L33-L37

https://github.com/nodejs/node/blob/af7164ebccd21d9fc5b0782e0427257f7637a4db/src/stream_wrap.cc#L303

I also found that @bnoordhuis had a PR before to implement uv_try_write for pipes but it didn't land finally.

I think implementing uv_try_write for pipes on Windows is the most elegant way to solve this issue if possible. Otherwise, we may need to prevent child process exit by calling procecss.exit() directly.

net.js and http2.js might have the same problem on Windows as they also call uv_try_write.