Restore default signal handling in child processes

In particular, this fixes Ctrl-C in nix-shell sessions.

7 files changed, 33 insertions, 35 deletions

diff --git a/src/download-via-ssh/download-via-ssh.cc b/src/download-via-ssh/download-via-ssh.cc
index ff28a60ff..4a1ba9a11 100644
--- a/src/download-via-ssh/download-via-ssh.cc
+++ b/src/download-via-ssh/download-via-ssh.cc
@@ -30,6 +30,7 @@ static std::pair<FdSink, FdSource> connect(const string & conn)
             throw SysError("dupping stdin");
         if (dup2(from.writeSide, STDOUT_FILENO) == -1)
             throw SysError("dupping stdout");
+        restoreSignals();
         execlp("ssh", "ssh", "-x", "-T", conn.c_str(), "nix-store --serve", NULL);
         throw SysError("executing ssh");
     });
diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc
index d564e0385..52cb23128 100644
--- a/src/libmain/shared.cc
+++ b/src/libmain/shared.cc
@@ -119,15 +119,9 @@ void initNix()
 
     startSignalHandlerThread();
 
-    /* Ignore SIGPIPE. */
+    /* Reset SIGCHLD to its default. */
     struct sigaction act;
     sigemptyset(&act.sa_mask);
-    act.sa_handler = SIG_IGN;
-    act.sa_flags = 0;
-    if (sigaction(SIGPIPE, &act, 0))
-        throw SysError("ignoring SIGPIPE");
-
-    /* Reset SIGCHLD to its default. */
     act.sa_handler = SIG_DFL;
     act.sa_flags = 0;
     if (sigaction(SIGCHLD, &act, 0))
@@ -252,7 +246,7 @@ void printVersion(const string & programName)
 
 void showManPage(const string & name)
 {
-    restoreSIGPIPE();
+    restoreSignals();
     execlp("man", "man", name.c_str(), NULL);
     throw SysError(format("command ‘man %1%’ failed") % name.c_str());
 }
@@ -305,16 +299,6 @@ RunPager::RunPager()
     if (!pager) pager = getenv("PAGER");
     if (pager && ((string) pager == "" || (string) pager == "cat")) return;
 
-    /* Ignore SIGINT. The pager will handle it (and we'll get
-       SIGPIPE). */
-    struct sigaction act;
-    act.sa_handler = SIG_IGN;
-    act.sa_flags = 0;
-    sigemptyset(&act.sa_mask);
-    if (sigaction(SIGINT, &act, 0)) throw SysError("ignoring SIGINT");
-
-    restoreSIGPIPE();
-
     Pipe toPager;
     toPager.create();
 
@@ -323,6 +307,7 @@ RunPager::RunPager()
             throw SysError("dupping stdin");
         if (!getenv("LESS"))
             setenv("LESS", "FRSXMK", 1);
+        restoreSignals();
         if (pager)
             execl("/bin/sh", "sh", "-c", pager, NULL);
         execlp("pager", "pager", NULL);
@@ -331,6 +316,8 @@ RunPager::RunPager()
         throw SysError(format("executing ‘%1%’") % pager);
     });
 
+    pid.setKillSignal(SIGINT);
+
     if (dup2(toPager.writeSide.get(), STDOUT_FILENO) == -1)
         throw SysError("dupping stdout");
 }
@@ -345,7 +332,11 @@ RunPager::~RunPager()
             pid.wait();
         }
     } catch (...) {
-        ignoreException();
+        try {
+            pid.kill(true);
+        } catch (...) {
+            ignoreException();
+        }
     }
 }
 
diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 7fb5271f4..40927c063 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -400,6 +400,8 @@ void Goal::trace(const format & f)
 /* Common initialisation performed in child processes. */
 static void commonChildInit(Pipe & logPipe)
 {
+    restoreSignals();
+
     /* Put the child in a separate session (and thus a separate
        process group) so that it has no controlling terminal (meaning
        that e.g. ssh cannot open /dev/tty) and it doesn't receive
@@ -2662,8 +2664,6 @@ void DerivationGoal::runChild()
         for (auto & i : drv->args)
             args.push_back(rewriteStrings(i, inputRewrites));
 
-        restoreSIGPIPE();
-
         /* Indicate that we managed to set up the build environment. */
         writeFull(STDERR_FILENO, string("\1\n"));
 
diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc
index 3d0159400..f5d0a2704 100644
--- a/src/libstore/ssh-store.cc
+++ b/src/libstore/ssh-store.cc
@@ -91,6 +91,7 @@ ref<RemoteStore::Connection> SSHStore::openConnection()
 {
     if ((pid_t) sshMaster == -1) {
         sshMaster = startProcess([&]() {
+            restoreSignals();
             if (key.empty())
                 execlp("ssh", "ssh", "-N", "-M", "-S", socketPath.c_str(), uri.c_str(), NULL);
             else
diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index ca4edc2cd..6c4c5c969 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -860,6 +860,8 @@ string runProgram(Path program, bool searchPath, const Strings & args,
         Strings args_(args);
         args_.push_front(program);
 
+        restoreSignals();
+
         if (searchPath)
             execvp(program.c_str(), stringsToCharPtrs(args_).data());
         else
@@ -909,16 +911,6 @@ void closeOnExec(int fd)
 }
 
 
-void restoreSIGPIPE()
-{
-    struct sigaction act;
-    act.sa_handler = SIG_DFL;
-    act.sa_flags = 0;
-    sigemptyset(&act.sa_mask);
-    if (sigaction(SIGPIPE, &act, 0)) throw SysError("resetting SIGPIPE");
-}
-
-
 //////////////////////////////////////////////////////////////////////
 
 
@@ -1218,19 +1210,31 @@ void triggerInterrupt()
     }
 }
 
+static sigset_t savedSignalMask;
+
 void startSignalHandlerThread()
 {
+    if (sigprocmask(SIG_BLOCK, nullptr, &savedSignalMask))
+        throw SysError("quering signal mask");
+
     sigset_t set;
     sigemptyset(&set);
     sigaddset(&set, SIGINT);
     sigaddset(&set, SIGTERM);
     sigaddset(&set, SIGHUP);
+    sigaddset(&set, SIGPIPE);
     if (pthread_sigmask(SIG_BLOCK, &set, nullptr))
         throw SysError("blocking signals");
 
     std::thread(signalHandlerThread, set).detach();
 }
 
+void restoreSignals()
+{
+    if (sigprocmask(SIG_SETMASK, &savedSignalMask, nullptr))
+        throw SysError("restoring signals");
+}
+
 /* RAII helper to automatically deregister a callback. */
 struct InterruptCallbackImpl : InterruptCallback
 {
diff --git a/src/libutil/util.hh b/src/libutil/util.hh
index 07141ffed..cfaaf1486 100644
--- a/src/libutil/util.hh
+++ b/src/libutil/util.hh
@@ -256,10 +256,6 @@ void closeMostFDs(const set<int> & exceptions);
 /* Set the close-on-exec flag for the given file descriptor. */
 void closeOnExec(int fd);
 
-/* Restore default handling of SIGPIPE, otherwise some programs will
-   randomly say "Broken pipe". */
-void restoreSIGPIPE();
-
 
 /* User interruption. */
 
@@ -423,6 +419,9 @@ void callSuccess(
    on the current thread (and thus any threads created by it). */
 void startSignalHandlerThread();
 
+/* Restore default signal handling. */
+void restoreSignals();
+
 struct InterruptCallback
 {
     virtual ~InterruptCallback() { };
diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc
index 3eb2d2c0b..ee030c57b 100755
--- a/src/nix-build/nix-build.cc
+++ b/src/nix-build/nix-build.cc
@@ -452,6 +452,8 @@ int main(int argc, char ** argv)
 
                 auto argPtrs = stringsToCharPtrs(args);
 
+                restoreSignals();
+
                 execvp(getEnv("NIX_BUILD_SHELL", "bash").c_str(), argPtrs.data());
 
                 throw SysError("executing shell");