From 818b33b60e94801226b2127053d4b20636da7f05 Mon Sep 17 00:00:00 2001
From: Ryan Dahl <ry@tinyclouds.org>
Date: Tue, 5 Oct 2010 09:51:50 -0700
Subject: [PATCH] child_process: Set CLOEXEC on the parent ends of the pipe

---
 src/node_child_process.cc            |    6 ++++++
 test/fixtures/fd-leaker-child.js     |   17 +++++++++++++++++
 test/fixtures/fd-leaker.js           |   32 ++++++++++++++++++++++++++++++++
 test/simple/test-stdio-fd-leaking.js |   25 +++++++++++++++++++++++++
 4 files changed, 80 insertions(+), 0 deletions(-)
 create mode 100644 test/fixtures/fd-leaker-child.js
 create mode 100644 test/fixtures/fd-leaker.js
 create mode 100644 test/simple/test-stdio-fd-leaking.js

diff --git a/src/node_child_process.cc b/src/node_child_process.cc
index 9201a91..2bd29e2 100644
--- a/src/node_child_process.cc
+++ b/src/node_child_process.cc
@@ -218,6 +218,12 @@ int ChildProcess::Spawn(const char *file,
     return -1;
   }

+  // Set CLOEXEC on the parent ends of the pipe, so we don't leak them into
+  // another child.
+  if (custom_fds[0] == -1) fcntl(stdin_pipe[1], F_SETFD, FD_CLOEXEC);
+  if (custom_fds[1] == -1) fcntl(stdout_pipe[0], F_SETFD, FD_CLOEXEC);
+  if (custom_fds[2] == -1) fcntl(stderr_pipe[0], F_SETFD, FD_CLOEXEC);
+
   // Save environ in the case that we get it clobbered
   // by the child process.
   char **save_our_env = environ;
diff --git a/test/fixtures/fd-leaker-child.js b/test/fixtures/fd-leaker-child.js
new file mode 100644
index 0000000..2fc1268
--- /dev/null
+++ b/test/fixtures/fd-leaker-child.js
@@ -0,0 +1,17 @@
+setInterval(function () {
+  console.log('hello');
+}, 500);
+
+process.stdout.on('error', function (e) {
+  console.error('Ignoring stdout error: %s', e.message);
+});
+
+var stdin = process.openStdin();
+
+stdin.on('error', function (e) {
+  console.error('Ignoring stdin error: %s', e.message);
+});
+
+stdin.on('end', function () {
+  process.exit(0);
+});
diff --git a/test/fixtures/fd-leaker.js b/test/fixtures/fd-leaker.js
new file mode 100644
index 0000000..fe36e61
--- /dev/null
+++ b/test/fixtures/fd-leaker.js
@@ -0,0 +1,32 @@
+var spawn = require('child_process').spawn;
+var join = require('path').join;
+var assert = require('assert');
+
+
+var childPath = join(__dirname, 'fd-leaker-child.js');
+
+var childA = spawn(process.execPath, [childPath]);
+console.log(childA.pid);
+
+// Start another child
+childA.stdout.setEncoding('utf8');
+
+var childB;
+
+childA.stdout.on('data', function (d) {
+  assert.equal('hello\n', d);
+
+  if (!childB) {
+    childB = spawn(process.execPath, [childPath]);
+    console.log(childB.pid);
+
+    childB.stdout.setEncoding('utf8');
+    childB.stdout.on('data', function (d) {
+      assert.equal('hello\n', d);
+      // Now commit suicide.
+      process.exit(0);
+      // Do the children live? They shouldn't.
+    });
+  }
+});
+
diff --git a/test/simple/test-stdio-fd-leaking.js b/test/simple/test-stdio-fd-leaking.js
new file mode 100644
index 0000000..6982970
--- /dev/null
+++ b/test/simple/test-stdio-fd-leaking.js
@@ -0,0 +1,25 @@
+var common = require('../common');
+var assert = require('assert');
+var spawn = require('child_process').spawn;
+
+// Example ptree:
+// 21347 test-stdio-fd-leaking.js (this script)
+//   21349 fd-leaker.js
+//     21361 fd-leaker-child.js
+//     21362 fd-leaker-child.js
+
+// fd-leaker starts two child processes: fd-leaker-child1, then 0.1 seconds
+// later fd-leaker-child2. It ouputs the PIDs of both child processes so
+// that this script knows them.
+//
+// fd-leaker-child prints, in an infinite loop 'hello' to stdout. It
+// ignores EPIPE errors on stdout and ONLY exits when stdin is closed.
+// It SHOULD exit when the parent exits, because the parent should close the
+// stdin implicitly.
+//
+// fd-leaker start two instances of fd-leaker-child so to demonstrate a bug.
+// The stdio pipes into the child processes are leaked into another so that
+// stdin is never actually closed...
+
+
+
--
1.7.2