Merge pull request #917 from zielmicha/vfork

osproc: Raise exception if execv fails in child process

lib/posix/linux.nim

@@ -0,0 +1,25 @@
+import posix
+
+const
+  CSIGNAL* = 0x000000FF
+  CLONE_VM* = 0x00000100
+  CLONE_FS* = 0x00000200
+  CLONE_FILES* = 0x00000400
+  CLONE_SIGHAND* = 0x00000800
+  CLONE_PTRACE* = 0x00002000
+  CLONE_VFORK* = 0x00004000
+  CLONE_PARENT* = 0x00008000
+  CLONE_THREAD* = 0x00010000
+  CLONE_NEWNS* = 0x00020000
+  CLONE_SYSVSEM* = 0x00040000
+  CLONE_SETTLS* = 0x00080000
+  CLONE_PARENT_SETTID* = 0x00100000
+  CLONE_CHILD_CLEARTID* = 0x00200000
+  CLONE_DETACHED* = 0x00400000
+  CLONE_UNTRACED* = 0x00800000
+  CLONE_CHILD_SETTID* = 0x01000000
+  CLONE_STOPPED* = 0x02000000
+
+# fn should be of type proc (a2: pointer): void {.cdecl.}
+proc clone*(fn: pointer; child_stack: pointer; flags: cint;
+            arg: pointer; ptid: ptr TPid; tls: pointer; ctid: ptr TPid): cint {.importc, header: "<sched.h>".}

lib/posix/posix.nim

@@ -2066,6 +2066,7 @@ proc pthread_spin_unlock*(a1: ptr Tpthread_spinlock): cint {.
 proc pthread_testcancel*() {.importc, header: "<pthread.h>".}
 
 
+proc exitnow*(code: int): void {.importc: "_exit", header: "<unistd.h>".}
 proc access*(a1: cstring, a2: cint): cint {.importc, header: "<unistd.h>".}
 proc alarm*(a1: cint): cint {.importc, header: "<unistd.h>".}
 proc chdir*(a1: cstring): cint {.importc, header: "<unistd.h>".}

lib/pure/osproc.nim

@@ -13,13 +13,18 @@
 include "system/inclrtl"
 
 import
-  strutils, os, strtabs, streams, sequtils
+  strutils, os, strtabs, streams
 
 when defined(windows):
   import winlean
 else:
   import posix
 
+when defined(linux):
+  import linux
+  when not defined(useFork):
+    const useClone = true
+
 type
   TProcess = object of TObject
     when defined(windows):
@@ -44,7 +49,7 @@ type
     poStdErrToStdOut,    ## merge stdout and stderr to the stdout stream
     poParentStreams      ## use the parent's streams
 
-template poUseShell*: TProcessOption {.deprecated.} = poUsePath
+const poUseShell* {.deprecated.} = poUsePath
   ## Deprecated alias for poUsePath.
 
 proc quoteShellWindows*(s: string): string {.noSideEffect, rtl, extern: "nosp$1".} =
@@ -590,6 +595,23 @@ elif not defined(useNimRtl):
       copyMem(result[i], addr(x[0]), x.len+1)
       inc(i)
 
+  type TStartProcessData = object
+    sysCommand: cstring
+    sysArgs: cstringArray
+    sysEnv: cstringArray
+    workingDir: cstring
+    pStdin, pStdout, pStderr, pErrorPipe: array[0..1, cint]
+    optionPoUsePath: bool
+    optionPoParentStreams: bool
+    optionPoStdErrToStdOut: bool
+
+  proc startProcessAuxSpawn(data: TStartProcessData): TPid {.tags: [FExecIO, FReadEnv].}
+  proc startProcessAuxFork(data: TStartProcessData): TPid {.tags: [FExecIO, FReadEnv].}
+  {.push stacktrace: off, profiler: off.}
+  proc startProcessAfterFork(data: ptr TStartProcessData) {.
+    tags: [FExecIO, FReadEnv], cdecl.}
+  {.pop.}
+
   proc startProcess(command: string,
                  workingDir: string = "",
                  args: openArray[string] = [],
@@ -604,100 +626,48 @@ elif not defined(useNimRtl):
          pipe(pStderr) != 0'i32:
         osError(osLastError())
 
-    var sys_command: string
+    var sysCommand: string
-    var sys_args_raw: seq[string]
+    var sysArgsRaw: seq[string]
     if poEvalCommand in options:
-      sys_command = "/bin/sh"
+      sysCommand = "/bin/sh"
-      sys_args_raw = @[sys_command, "-c", command]
+      sysArgsRaw = @[sysCommand, "-c", command]
       assert args.len == 0
     else:
-      sys_command = command
+      sysCommand = command
-      sys_args_raw = @[command]
+      sysArgsRaw = @[command]
       for arg in args.items:
-        sys_args_raw.add arg
+        sysArgsRaw.add arg
-
-    var sys_args = allocCStringArray(sys_args_raw)
-    finally: deallocCStringArray(sys_args)
 
     var pid: TPid
-    when defined(posix_spawn) and not defined(useFork):
-      var attr: Tposix_spawnattr
-      var fops: Tposix_spawn_file_actions
 
-      template chck(e: expr) =
+    var sysArgs = allocCStringArray(sysArgsRaw)
-        if e != 0'i32: osError(osLastError())
+    finally: deallocCStringArray(sysArgs)
 
-      chck posix_spawn_file_actions_init(fops)
+    var sysEnv = if env == nil:
-      chck posix_spawnattr_init(attr)
+        envToCStringArray()
-
-      var mask: Tsigset
-      chck sigemptyset(mask)
-      chck posix_spawnattr_setsigmask(attr, mask)
-      chck posix_spawnattr_setpgroup(attr, 0'i32)
-
-      chck posix_spawnattr_setflags(attr, POSIX_SPAWN_USEVFORK or
-                                          POSIX_SPAWN_SETSIGMASK or
-                                          POSIX_SPAWN_SETPGROUP)
-
-      if poParentStreams notin options:
-        chck posix_spawn_file_actions_addclose(fops, pStdin[writeIdx])
-        chck posix_spawn_file_actions_adddup2(fops, pStdin[readIdx], readIdx)
-        chck posix_spawn_file_actions_addclose(fops, pStdout[readIdx])
-        chck posix_spawn_file_actions_adddup2(fops, pStdout[writeIdx], writeIdx)
-        chck posix_spawn_file_actions_addclose(fops, pStderr[readIdx])
-        if poStdErrToStdOut in options:
-          chck posix_spawn_file_actions_adddup2(fops, pStdout[writeIdx], 2)
-        else:
-          chck posix_spawn_file_actions_adddup2(fops, p_stderr[writeIdx], 2)
-
-      var sys_env = if env == nil: envToCStringArray() else: envToCStringArray(env)
-      var res: cint
-      # This is incorrect!
-      if workingDir.len > 0: os.setCurrentDir(workingDir)
-      if poUsePath in options:
-        res = posix_spawnp(pid, sys_command, fops, attr, sys_args, sys_env)
       else:
-        res = posix_spawn(pid, sys_command, fops, attr, sys_args, sys_env)
+        envToCStringArray(env)
-      deallocCStringArray(sys_env)
-      discard posix_spawn_file_actions_destroy(fops)
-      discard posix_spawnattr_destroy(attr)
-      chck res
 
+    finally: deallocCStringArray(sysEnv)
+
+    var data: TStartProcessData
+    data.sysCommand = sysCommand
+    data.sysArgs = sysArgs
+    data.sysEnv = sysEnv
+    data.pStdin = pStdin
+    data.pStdout = pStdout
+    data.pStderr = pStderr
+    data.optionPoParentStreams = poParentStreams in options
+    data.optionPoUsePath = poUsePath in options
+    data.optionPoStdErrToStdOut = poStdErrToStdOut in options
+    data.workingDir = workingDir
+
+
+    when defined(posix_spawn) and not defined(useFork) and not defined(useClone):
+      pid = startProcessAuxSpawn(data)
     else:
-      pid = fork()
+      pid = startProcessAuxFork(data)
-      if pid < 0: osError(osLastError())
-      if pid == 0:
-        ## child process:
 
-        if poParentStreams notin options:
-          discard close(p_stdin[writeIdx])
-          if dup2(p_stdin[readIdx], readIdx) < 0: osError(osLastError())
-          discard close(p_stdout[readIdx])
-          if dup2(p_stdout[writeIdx], writeIdx) < 0: osError(osLastError())
-          discard close(p_stderr[readIdx])
-          if poStdErrToStdOut in options:
-            if dup2(p_stdout[writeIdx], 2) < 0: osError(osLastError())
-          else:
-            if dup2(p_stderr[writeIdx], 2) < 0: osError(osLastError())
-
-        # Create a new process group
-        if setpgid(0, 0) == -1: quit("setpgid call failed: " & $strerror(errno))
-
-        if workingDir.len > 0: os.setCurrentDir(workingDir)
-
-        if env == nil:
-          if poUsePath in options:
-            discard execvp(sys_command, sys_args)
-          else:
-            discard execv(sys_command, sys_args)
-        else:
-          var c_env = envToCStringArray(env)
-          if poUsePath in options:
-            discard execvpe(sys_command, sys_args, c_env)
-          else:
-            discard execve(sys_command, sys_args, c_env)
-        # too risky to raise an exception here:
-        quit("execve call failed: " & $strerror(errno))
     # Parent process. Copy process information.
     if poEchoCmd in options:
       echo(command, " ", join(args, " "))
@@ -723,6 +693,128 @@ elif not defined(useNimRtl):
       discard close(pStdin[readIdx])
       discard close(pStdout[writeIdx])
 
+  proc startProcessAuxSpawn(data: TStartProcessData): TPid =
+    var attr: Tposix_spawnattr
+    var fops: Tposix_spawn_file_actions
+
+    template chck(e: expr) =
+      if e != 0'i32: osError(osLastError())
+
+    chck posix_spawn_file_actions_init(fops)
+    chck posix_spawnattr_init(attr)
+
+    var mask: Tsigset
+    chck sigemptyset(mask)
+    chck posix_spawnattr_setsigmask(attr, mask)
+    chck posix_spawnattr_setpgroup(attr, 0'i32)
+
+    chck posix_spawnattr_setflags(attr, POSIX_SPAWN_USEVFORK or
+                                        POSIX_SPAWN_SETSIGMASK or
+                                        POSIX_SPAWN_SETPGROUP)
+
+    if not data.optionPoParentStreams:
+      chck posix_spawn_file_actions_addclose(fops, data.pStdin[writeIdx])
+      chck posix_spawn_file_actions_adddup2(fops, data.pStdin[readIdx], readIdx)
+      chck posix_spawn_file_actions_addclose(fops, data.pStdout[readIdx])
+      chck posix_spawn_file_actions_adddup2(fops, data.pStdout[writeIdx], writeIdx)
+      chck posix_spawn_file_actions_addclose(fops, data.pStderr[readIdx])
+      if data.optionPoStdErrToStdOut:
+        chck posix_spawn_file_actions_adddup2(fops, data.pStdout[writeIdx], 2)
+      else:
+        chck posix_spawn_file_actions_adddup2(fops, data.pStderr[writeIdx], 2)
+
+    var res: cint
+    # FIXME: chdir is global to process
+    if data.workingDir.len > 0:
+      setCurrentDir($data.workingDir)
+    var pid: TPid
+
+    if data.optionPoUsePath:
+      res = posix_spawnp(pid, data.sysCommand, fops, attr, data.sysArgs, data.sysEnv)
+    else:
+      res = posix_spawn(pid, data.sysCommand, fops, attr, data.sysArgs, data.sysEnv)
+
+    discard posix_spawn_file_actions_destroy(fops)
+    discard posix_spawnattr_destroy(attr)
+    chck res
+    return pid
+
+  proc startProcessAuxFork(data: TStartProcessData): TPid =
+    if pipe(data.pErrorPipe) != 0:
+        osError(osLastError())
+
+    finally:
+      discard close(data.pErrorPipe[readIdx])
+
+    var pid: TPid
+    var dataCopy = data
+
+    if defined(useClone):
+      const stackSize = 8096
+      let stackEnd = cast[clong](alloc(stackSize))
+      let stack = cast[pointer](stackEnd + stackSize)
+      let fn: pointer = startProcessAfterFork
+      pid = clone(fn, stack,
+                  cint(CLONE_VM or CLONE_VFORK or SIGCHLD),
+                  pointer(addr dataCopy), nil, nil, nil)
+      discard close(data.pErrorPipe[writeIdx])
+      dealloc(stack)
+    else:
+      pid = fork()
+      if pid == 0:
+        startProcessAfterFork(addr(dataCopy))
+        exitnow(1)
+
+    discard close(data.pErrorPipe[writeIdx])
+    if pid < 0: osError(osLastError())
+
+    var error: cint
+    let sizeRead = read(data.pErrorPipe[readIdx], addr error, sizeof(error))
+    if sizeRead == sizeof(error):
+      osError($strerror(error))
+
+    return pid
+
+  {.push stacktrace: off, profiler: off.}
+  proc startProcessFail(data: ptr TStartProcessData) =
+    var error: cint = errno
+    discard write(data.pErrorPipe[writeIdx], addr error, sizeof(error))
+    exitnow(1)
+
+  proc startProcessAfterFork(data: ptr TStartProcessData) =
+    # Warning: no GC here!
+    # Or anythink that touches global structures - all called nimrod procs
+    # must be marked with noStackFrame. Inspect C code after making changes.
+    if not data.optionPoParentStreams:
+      discard close(data.pStdin[writeIdx])
+      if dup2(data.pStdin[readIdx], readIdx) < 0:
+        startProcessFail(data)
+      discard close(data.pStdout[readIdx])
+      if dup2(data.pStdout[writeIdx], writeIdx) < 0:
+        startProcessFail(data)
+      discard close(data.pStderr[readIdx])
+      if data.optionPoStdErrToStdOut:
+        if dup2(data.pStdout[writeIdx], 2) < 0:
+          startProcessFail(data)
+      else:
+        if dup2(data.pStderr[writeIdx], 2) < 0:
+          startProcessFail(data)
+
+    if data.workingDir.len > 0:
+      if chdir(data.workingDir) < 0:
+        startProcessFail(data)
+
+    discard close(data.pErrorPipe[readIdx])
+    discard fcntl(data.pErrorPipe[writeIdx], F_SETFD, FD_CLOEXEC)
+
+    if data.optionPoUsePath:
+      discard execvpe(data.sysCommand, data.sysArgs, data.sysEnv)
+    else:
+      discard execve(data.sysCommand, data.sysArgs, data.sysEnv)
+
+    startProcessFail(data)
+  {.pop}
+
   proc close(p: PProcess) =
     if p.inStream != nil: close(p.inStream)
     if p.outStream != nil: close(p.outStream)