Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Set FD_CLOEXEC flag on stdio FDs before spawning.

With regression test.
  • Loading branch information...
commit 07da49b095553bf440dab05072755343afbdc461 1 parent a3750a9
@guitt guitt authored ry committed
View
26 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;
View
71 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');
+});
Please sign in to comment.
Something went wrong with that request. Please try again.