POSIX: detach background subprocesses from terminal.
authorJulien Tinnes <jln@chromium.org>
Thu, 29 Jan 2015 19:33:35 +0000 (11:33 -0800)
committerJulien Tinnes <jln@chromium.org>
Thu, 29 Jan 2015 19:36:30 +0000 (11:36 -0800)
Put background subprocesses (i.e. subprocesses with no access
to the console) in their own session and detach them from the
terminal.

This fixes martine/ninja#909.

src/subprocess-posix.cc
src/subprocess_test.cc

index 40c9ae1..cc8bff6 100644 (file)
@@ -80,8 +80,11 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) {
         break;
 
       if (!use_console_) {
-        // Put the child in its own process group, so ctrl-c won't reach it.
-        if (setpgid(0, 0) < 0)
+        // Put the child in its own session and process group. It will be
+        // detached from the current terminal and ctrl-c won't reach it.
+        // Since this process was just forked, it is not a process group leader
+        // and setsid() will succeed.
+        if (setsid() < 0)
           break;
 
         // Open /dev/null over stdin.
index 8a0787c..76f5c6c 100644 (file)
@@ -16,6 +16,8 @@
 
 #include "test.h"
 
+#include <string>
+
 #ifndef _WIN32
 // SetWithLots need setrlimit.
 #include <stdio.h>
@@ -96,12 +98,23 @@ TEST_F(SubprocessTest, InterruptParent) {
   ASSERT_FALSE("We should have been interrupted");
 }
 
+// A shell command to check if the current process is connected to a terminal.
+// This is different from having stdin/stdout/stderr be a terminal. (For
+// instance consider the command "yes < /dev/null > /dev/null 2>&1".
+// As "ps" will confirm, "yes" could still be connected to a terminal, despite
+// not having any of the standard file descriptors be a terminal.
+static const char kIsConnectedToTerminal[] = "tty < /dev/tty > /dev/null";
+
 TEST_F(SubprocessTest, Console) {
   // Skip test if we don't have the console ourselves.
   if (isatty(0) && isatty(1) && isatty(2)) {
-    Subprocess* subproc = subprocs_.Add("test -t 0 -a -t 1 -a -t 2",
-                                        /*use_console=*/true);
-    ASSERT_NE((Subprocess *) 0, subproc);
+    // Test that stdin, stdout and stderr are a terminal.
+    // Also check that the current process is connected to a terminal.
+    Subprocess* subproc =
+        subprocs_.Add(std::string("test -t 0 -a -t 1 -a -t 2 && ") +
+                          std::string(kIsConnectedToTerminal),
+                      /*use_console=*/true);
+    ASSERT_NE((Subprocess*)0, subproc);
 
     while (!subproc->Done()) {
       subprocs_.DoWork();
@@ -111,6 +124,18 @@ TEST_F(SubprocessTest, Console) {
   }
 }
 
+TEST_F(SubprocessTest, NoConsole) {
+  Subprocess* subproc =
+      subprocs_.Add(kIsConnectedToTerminal, /*use_console=*/false);
+  ASSERT_NE((Subprocess*)0, subproc);
+
+  while (!subproc->Done()) {
+    subprocs_.DoWork();
+  }
+
+  EXPECT_NE(ExitSuccess, subproc->Finish());
+}
+
 #endif
 
 TEST_F(SubprocessTest, SetWithSingle) {