Fix issue 21575 - Child processes spawned by std.process.spawnProcess accidentally inherit signal masks in parent process (#7766)

Fix issue 21575 - Child processes spawned by std.process.spawnProcess accidentally inherit signal masks in parent process
merged-on-behalf-of: unknown

std/process.d

@@ -873,6 +873,7 @@ version (Posix) private enum InternalError : ubyte
     getrlimit,
     doubleFork,
     malloc,
+    preExec,
 }
 
 /*
@@ -913,7 +914,7 @@ private Pid spawnProcessPosix(scope const(char[])[] args,
     argz[$-1] = null;
 
     // Prepare environment.
-    auto envz = createEnv(env, !(config & Config.newEnv));
+    auto envz = createEnv(env, !(config.flags & Config.Flags.newEnv));
 
     // Open the working directory.
     // We use open in the parent and fchdir in the child
@@ -960,13 +961,13 @@ private Pid spawnProcessPosix(scope const(char[])[] args,
     since the first and the second forks will run in parallel.
     */
     int[2] pidPipe;
-    if (config & Config.detached)
+    if (config.flags & Config.Flags.detached)
     {
         if (core.sys.posix.unistd.pipe(pidPipe) != 0)
             throw ProcessException.newFromErrno("Could not create pipe to get process pid");
         setCLOEXEC(pidPipe[1], true);
     }
-    scope(exit) if (config & Config.detached) close(pidPipe[0]);
+    scope(exit) if (config.flags & Config.Flags.detached) close(pidPipe[0]);
 
     static void abortOnError(int forkPipeOut, InternalError errorType, int error) nothrow
     {
@@ -980,7 +981,7 @@ private Pid spawnProcessPosix(scope const(char[])[] args,
     void closePipeWriteEnds()
     {
         close(forkPipe[1]);
-        if (config & Config.detached)
+        if (config.flags & Config.Flags.detached)
             close(pidPipe[1]);
     }
 
@@ -998,7 +999,7 @@ private Pid spawnProcessPosix(scope const(char[])[] args,
         // Child process
 
         // no need for the read end of pipe on child side
-        if (config & Config.detached)
+        if (config.flags & Config.Flags.detached)
             close(pidPipe[0]);
         close(forkPipe[0]);
         immutable forkPipeOut = forkPipe[1];
@@ -1033,7 +1034,7 @@ private Pid spawnProcessPosix(scope const(char[])[] args,
             setCLOEXEC(STDOUT_FILENO, false);
             setCLOEXEC(STDERR_FILENO, false);
 
-            if (!(config & Config.inheritFDs))
+            if (!(config.flags & Config.Flags.inheritFDs))
             {
                 // NOTE: malloc() and getrlimit() are not on the POSIX async
                 // signal safe functions list, but practically this should
@@ -1095,6 +1096,14 @@ private Pid spawnProcessPosix(scope const(char[])[] args,
                 if (stderrFD > STDERR_FILENO)  close(stderrFD);
             }
 
+            if (config.preExecFunction !is null)
+            {
+                if (config.preExecFunction() != true)
+                {
+                    abortOnError(forkPipeOut, InternalError.preExec, .errno);
+                }
+            }
+
             // Execute program.
             core.sys.posix.unistd.execve(argz[0], argz.ptr, envz);
 
@@ -1102,7 +1111,7 @@ private Pid spawnProcessPosix(scope const(char[])[] args,
             abortOnError(forkPipeOut, InternalError.exec, .errno);
         }
 
-        if (config & Config.detached)
+        if (config.flags & Config.Flags.detached)
         {
             auto secondFork = core.sys.posix.unistd.fork();
             if (secondFork == 0)
@@ -1143,7 +1152,7 @@ private Pid spawnProcessPosix(scope const(char[])[] args,
         // Save error number just in case if subsequent "waitpid" fails and overrides errno
         immutable lastError = .errno;
 
-        if (config & Config.detached)
+        if (config.flags & Config.Flags.detached)
         {
             // Forked child exits right after creating second fork. So it should be safe to wait here.
             import core.sys.posix.sys.wait : waitpid;
@@ -1173,12 +1182,15 @@ private Pid spawnProcessPosix(scope const(char[])[] args,
                     break;
                 case InternalError.doubleFork:
                     // Can happen only when starting detached process
-                    assert(config & Config.detached);
+                    assert(config.flags & Config.Flags.detached);
                     errorMsg = "Failed to fork twice";
                     break;
                 case InternalError.malloc:
                     errorMsg = "Failed to allocate memory";
                     break;
+                case InternalError.preExec:
+                    errorMsg = "Failed to execute preExecFunction";
+                    break;
                 case InternalError.noerror:
                     assert(false);
             }
@@ -1186,7 +1198,7 @@ private Pid spawnProcessPosix(scope const(char[])[] args,
                 throw ProcessException.newFromErrno(error, errorMsg);
             throw new ProcessException(errorMsg);
         }
-        else if (config & Config.detached)
+        else if (config.flags & Config.Flags.detached)
         {
             owned = false;
             if (read(pidPipe[0], &id, id.sizeof) != id.sizeof)
@@ -1194,19 +1206,73 @@ private Pid spawnProcessPosix(scope const(char[])[] args,
         }
 
         // Parent process:  Close streams and return.
-        if (!(config & Config.retainStdin ) && stdinFD  > STDERR_FILENO
+        if (!(config.flags & Config.Flags.retainStdin ) && stdinFD  > STDERR_FILENO
                                             && stdinFD  != getFD(std.stdio.stdin ))
             stdin.close();
-        if (!(config & Config.retainStdout) && stdoutFD > STDERR_FILENO
+        if (!(config.flags & Config.Flags.retainStdout) && stdoutFD > STDERR_FILENO
                                             && stdoutFD != getFD(std.stdio.stdout))
             stdout.close();
-        if (!(config & Config.retainStderr) && stderrFD > STDERR_FILENO
+        if (!(config.flags & Config.Flags.retainStderr) && stderrFD > STDERR_FILENO
                                             && stderrFD != getFD(std.stdio.stderr))
             stderr.close();
         return new Pid(id, owned);
     }
 }
 
+version (Posix)
+@system unittest
+{
+    import std.concurrency : ownerTid, receiveTimeout, send, spawn;
+    import std.datetime : seconds;
+
+    sigset_t ss;
+    sigemptyset(&ss);
+    sigaddset(&ss, SIGINT);
+    pthread_sigmask(SIG_BLOCK, &ss, null);
+
+    Config config = {
+        preExecFunction: () @trusted @nogc nothrow {
+            // Reset signal handlers
+            sigset_t ss;
+            if (sigfillset(&ss) != 0)
+            {
+                return false;
+            }
+            if (sigprocmask(SIG_UNBLOCK, &ss, null) != 0)
+            {
+                return false;
+            }
+            return true;
+        },
+    };
+
+    auto pid = spawnProcess(["sleep", "10000"],
+                            std.stdio.stdin,
+                            std.stdio.stdout,
+                            std.stdio.stderr,
+                            null,
+                            config,
+                            null);
+    scope(failure)
+    {
+        kill(pid, SIGKILL);
+        wait(pid);
+    }
+
+    // kill the spawned process with SIGINT
+    // and send its return code
+    spawn((shared Pid pid) {
+        auto p = cast() pid;
+        kill(p, SIGINT);
+        auto code = wait(p);
+        assert(code < 0);
+        send(ownerTid, code);
+    }, cast(shared) pid);
+
+    auto received = receiveTimeout(3.seconds, (int) {});
+    assert(received);
+}
+
 /*
 Implementation of spawnProcess() for Windows.
 
@@ -1233,7 +1299,7 @@ private Pid spawnProcessWin(scope const(char)[] commandLine,
     if (commandLine.empty) throw new RangeError("Command line is empty");
 
     // Prepare environment.
-    auto envz = createEnv(env, !(config & Config.newEnv));
+    auto envz = createEnv(env, !(config.flags & Config.Flags.newEnv));
 
     // Startup info for CreateProcessW().
     STARTUPINFO_W startinfo;
@@ -1285,7 +1351,7 @@ private Pid spawnProcessWin(scope const(char)[] commandLine,
     PROCESS_INFORMATION pi;
     DWORD dwCreationFlags =
         CREATE_UNICODE_ENVIRONMENT |
-        ((config & Config.suppressConsole) ? CREATE_NO_WINDOW : 0);
+        ((config.flags & Config.Flags.suppressConsole) ? CREATE_NO_WINDOW : 0);
     // workaround until https://issues.dlang.org/show_bug.cgi?id=14696 is fixed
     auto pworkDir = workDir.tempCStringW();
     if (!CreateProcessW(null, commandLine.tempCStringW().buffPtr,
@@ -1294,19 +1360,19 @@ private Pid spawnProcessWin(scope const(char)[] commandLine,
         throw ProcessException.newFromLastError("Failed to spawn process \"" ~ cast(string) program ~ '"');
 
     // figure out if we should close any of the streams
-    if (!(config & Config.retainStdin ) && stdinFD  > STDERR_FILENO
+    if (!(config.flags & Config.Flags.retainStdin ) && stdinFD  > STDERR_FILENO
                                         && stdinFD  != getFD(std.stdio.stdin ))
         stdin.close();
-    if (!(config & Config.retainStdout) && stdoutFD > STDERR_FILENO
+    if (!(config.flags & Config.Flags.retainStdout) && stdoutFD > STDERR_FILENO
                                         && stdoutFD != getFD(std.stdio.stdout))
         stdout.close();
-    if (!(config & Config.retainStderr) && stderrFD > STDERR_FILENO
+    if (!(config.flags & Config.Flags.retainStderr) && stderrFD > STDERR_FILENO
                                         && stderrFD != getFD(std.stdio.stderr))
         stderr.close();
 
     // close the thread handle in the process info structure
     CloseHandle(pi.hThread);
-    if (config & Config.detached)
+    if (config.flags & Config.Flags.detached)
     {
         CloseHandle(pi.hProcess);
         return new Pid(pi.dwProcessId);
@@ -1660,7 +1726,7 @@ version (Posix) @system unittest
         pipei.writeEnd.flush();
         assert(pipeo.readEnd.readln().chomp() == "input output foo");
         assert(pipee.readEnd.readln().chomp().stripRight() == "input error bar");
-        if (config & Config.detached)
+        if (config.flags & Config.Flags.detached)
             while (!done.exists) Thread.sleep(10.msecs);
         else
             wait(pid);
@@ -1680,7 +1746,7 @@ version (Posix) @system unittest
         auto filee = File(pathe, "w");
         auto done = buildPath(tempDir(), randomUUID().toString());
         auto pid = spawnProcess([prog.path, "bar", "baz", done], filei, fileo, filee, null, config);
-        if (config & Config.detached)
+        if (config.flags & Config.Flags.detached)
             while (!done.exists) Thread.sleep(10.msecs);
         else
             wait(pid);
@@ -1953,12 +2019,10 @@ version (Windows)
 
 
 /**
-Flags that control the behaviour of process creation functions in this
+Options that control the behaviour of process creation functions in this
-module. Most flags only apply to $(LREF spawnProcess) and
+module. Most options only apply to $(LREF spawnProcess) and
 $(LREF spawnShell).
 
-Use bitwise OR to combine flags.
-
 Example:
 ---
 auto logFile = File("myapp_error.log", "w");
@@ -1976,7 +2040,13 @@ scope(exit)
 }
 ---
 */
-enum Config
+struct Config
+{
+    /**
+       Flag options.
+       Use bitwise OR to combine flags.
+    **/
+    enum Flags
     {
         none = 0,
 
@@ -2045,7 +2115,41 @@ enum Config
         */
         stderrPassThrough = 128,
     }
+    Flags flags; /// ditto
 
+    /**
+       For backwards compatibility, and cases when only flags need to
+       be specified in the `Config`, these allow building `Config`
+       instances using flag names only.
+    */
+    enum Config none = Config.init;
+    enum Config newEnv = Config(Flags.newEnv); /// ditto
+    enum Config retainStdin = Config(Flags.retainStdin); /// ditto
+    enum Config retainStdout = Config(Flags.retainStdout); /// ditto
+    enum Config retainStderr = Config(Flags.retainStderr); /// ditto
+    enum Config suppressConsole = Config(Flags.suppressConsole); /// ditto
+    enum Config inheritFDs = Config(Flags.inheritFDs); /// ditto
+    enum Config detached = Config(Flags.detached); /// ditto
+    enum Config stderrPassThrough = Config(Flags.stderrPassThrough); /// ditto
+    Config opBinary(string op : "|")(Config other)
+    {
+        return Config(flags | other.flags);
+    } /// ditto
+
+    version (StdDdoc)
+    {
+        /**
+        A function that is called before `exec` in $(LREF spawnProcess).
+        It returns `true` if succeeded and otherwise returns `false`.
+        On Windows, this member is not available.
+        */
+        bool function() nothrow @nogc @safe preExecFunction;
+    }
+    else version (Posix)
+    {
+        bool function() nothrow @nogc @safe preExecFunction;
+    }
+}
 
 /// A handle that corresponds to a spawned process.
 final class Pid
@@ -2808,7 +2912,7 @@ private ProcessPipes pipeProcessImpl(alias spawnFunc, Cmd, ExtraSpawnFuncArgs...
         childStderr = childStdout;
     }
 
-    config &= ~(Config.retainStdin | Config.retainStdout | Config.retainStderr);
+    config.flags &= ~(Config.Flags.retainStdin | Config.Flags.retainStdout | Config.Flags.retainStderr);
     pipes._pid = spawnFunc(command, childStdin, childStdout, childStderr,
                            env, config, workDir, extraArgs);
     return pipes;
@@ -3099,7 +3203,7 @@ private auto executeImpl(alias pipeFunc, Cmd, ExtraPipeFuncArgs...)(
     import std.array : appender;
     import std.typecons : Tuple;
 
-    auto redirect = (config & Config.stderrPassThrough)
+    auto redirect = (config.flags & Config.Flags.stderrPassThrough)
         ? Redirect.stdout
         : Redirect.stdout | Redirect.stderrToStdout;