diff --git a/src/node_child_process.cc b/src/node_child_process.cc index 347ee026a7..2237f409c1 100644 --- a/src/node_child_process.cc +++ b/src/node_child_process.cc @@ -39,6 +39,16 @@ static inline int SetNonBlocking(int fd) { } +static inline int SetCloseOnExec(int fd) { + int flags = fcntl(fd, F_GETFD, 0); + int r = fcntl(fd, F_SETFD, flags | FD_CLOEXEC); + if (r != 0) { + perror("SetCloseOnExec()"); + } + return r; +} + + static inline int ResetFlags(int fd) { int flags = fcntl(fd, F_GETFL, 0); // blocking @@ -223,6 +233,22 @@ int ChildProcess::Spawn(const char *file, return -1; } + // Set the close-on-exec FD flag + if (custom_fds[0] == -1) { + SetCloseOnExec(stdin_pipe[0]); + SetCloseOnExec(stdin_pipe[1]); + } + + if (custom_fds[1] == -1) { + SetCloseOnExec(stdout_pipe[0]); + SetCloseOnExec(stdout_pipe[1]); + } + + if (custom_fds[2] == -1) { + SetCloseOnExec(stderr_pipe[0]); + SetCloseOnExec(stderr_pipe[1]); + } + // Save environ in the case that we get it clobbered // by the child process. char **save_our_env = environ; diff --git a/test/simple/test-child-process-double-pipe.js b/test/simple/test-child-process-double-pipe.js new file mode 100644 index 0000000000..f98cb65ff1 --- /dev/null +++ b/test/simple/test-child-process-double-pipe.js @@ -0,0 +1,71 @@ +var assert = require('assert'), + util = require('util'), + spawn = require('child_process').spawn; + +// We're trying to reproduce: +// $ echo -e "hello\nnode\nand\nworld" | grep o | sed s/o/a/ + +var + echo = spawn('echo', ['-e', 'hello\nnode\nand\nworld\n']), + grep = spawn('grep', ['o']), + sed = spawn('sed', ['s/o/O/']); + +/* + * grep and sed hang if the spawn function leaks file descriptors to child + * processes. + * This happens when calling pipe(2) and then forgetting to set the + * FD_CLOEXEC flag on the resulting file descriptors. + * + * This test checks child processes exit, meaning they don't hang like + * explained above. + */ + + + +// pipe echo | grep +echo.stdout.on('data', function (data) { + if (!grep.stdin.write(data)) { + echo.stdout.pause(); + } +}); + +grep.stdin.on('drain', function (data) { + echo.stdout.resume(); +}); + +// propagate end from echo to grep +echo.stdout.on('end', function (code) { + grep.stdin.end(); +}); + + + +// pipe grep | sed +grep.stdout.on('data', function (data) { + if (!sed.stdin.write(data)) { + grep.stdout.pause(); + } +}); + +sed.stdin.on('drain', function (data) { + grep.stdout.resume(); +}); + +// propagate end from grep to sed +grep.stdout.on('end', function (code) { + sed.stdin.end(); +}); + + + +var result = ''; + +// print sed's output +sed.stdout.on('data', function (data) { + result += data.toString('utf8', 0, data.length); + util.print(data); +}); + +sed.stdout.on('end', function (code) { + assert.equal(result, 'hellO\nnOde\nwOrld\n'); +});