WIFSIGNALED means process has exited too (with a bang!) (#5678)

6 files changed, 95 insertions, 18 deletions

diff --git a/lib/posix/posix.nim b/lib/posix/posix.nim
index ab608efc6..02312a4d5 100644
--- a/lib/posix/posix.nim
+++ b/lib/posix/posix.nim
@@ -97,15 +97,6 @@ when not defined(macosx):
     ## Second-granularity time of last status change.
     result = s.st_ctim.tv_sec
 
-proc WIFCONTINUED*(s:cint) : bool {.importc, header: "<sys/wait.h>".}
-  ## True if child has been continued.
-proc WIFEXITED*(s:cint) : bool {.importc, header: "<sys/wait.h>".}
-  ## True if child exited normally.
-proc WIFSIGNALED*(s:cint) : bool {.importc, header: "<sys/wait.h>".}
-  ## True if child exited due to uncaught signal.
-proc WIFSTOPPED*(s:cint) : bool {.importc, header: "<sys/wait.h>".}
-  ## True if child is currently stopped.
-
 when hasAioH:
   proc aio_cancel*(a1: cint, a2: ptr Taiocb): cint {.importc, header: "<aio.h>".}
   proc aio_error*(a1: ptr Taiocb): cint {.importc, header: "<aio.h>".}
diff --git a/lib/posix/posix_linux_amd64.nim b/lib/posix/posix_linux_amd64.nim
index 4948642c1..70f7e710f 100644
--- a/lib/posix/posix_linux_amd64.nim
+++ b/lib/posix/posix_linux_amd64.nim
@@ -602,3 +602,12 @@ var
 include posix_linux_amd64_consts
 
 const POSIX_SPAWN_USEVFORK* = cint(0x40)  # needs _GNU_SOURCE!
+
+# <sys/wait.h>
+proc WEXITSTATUS*(s: cint): cint =  (s and 0xff00) shr 8
+proc WTERMSIG*(s:cint): cint = s and 0x7f
+proc WSTOPSIG*(s:cint): cint = WEXITSTATUS(s)
+proc WIFEXITED*(s:cint) : bool = WTERMSIG(s) == 0
+proc WIFSIGNALED*(s:cint) : bool = (cast[int8]((s and 0x7f) + 1) shr 1) > 0
+proc WIFSTOPPED*(s:cint) : bool = (s and 0xff) == 0x7f
+proc WIFCONTINUED*(s:cint) : bool = s == W_CONTINUED
diff --git a/lib/posix/posix_other.nim b/lib/posix/posix_other.nim
index 6e1c33605..b1cc244b7 100644
--- a/lib/posix/posix_other.nim
+++ b/lib/posix/posix_other.nim
@@ -611,3 +611,18 @@ when hasSpawnH:
     # OR'ing of flags:
     const POSIX_SPAWN_USEVFORK* = cint(0)
 
+# <sys/wait.h>
+proc WEXITSTATUS*(s: cint): cint {.importc, header: "<sys/wait.h>".}
+  ## Exit code, iff WIFEXITED(s)
+proc WTERMSIG*(s: cint): cint {.importc, header: "<sys/wait.h>".}
+  ## Termination signal, iff WIFSIGNALED(s)
+proc WSTOPSIG*(s: cint): cint {.importc, header: "<sys/wait.h>".}
+  ## Stop signal, iff WIFSTOPPED(s)
+proc WIFEXITED*(s: cint): bool {.importc, header: "<sys/wait.h>".}
+  ## True if child exited normally.
+proc WIFSIGNALED*(s: cint): bool {.importc, header: "<sys/wait.h>".}
+  ## True if child exited due to uncaught signal.
+proc WIFSTOPPED*(s: cint): bool {.importc, header: "<sys/wait.h>".}
+  ## True if child is currently stopped.
+proc WIFCONTINUED*(s: cint): bool {.importc, header: "<sys/wait.h>".}
+  ## True if child has been continued.
diff --git a/lib/pure/osproc.nim b/lib/pure/osproc.nim
index 0f37f8fe0..82a0c0c65 100644
--- a/lib/pure/osproc.nim
+++ b/lib/pure/osproc.nim
@@ -209,9 +209,16 @@ proc waitForExit*(p: Process, timeout: int = -1): int {.rtl,
   ##
   ## **Warning**: Be careful when using waitForExit for processes created without
   ## poParentStreams because they may fill output buffers, causing deadlock.
+  ##
+  ## On posix, if the process has exited because of a signal, 128 + signal
+  ## number will be returned.
+
 
 proc peekExitCode*(p: Process): int {.tags: [].}
   ## return -1 if the process is still running. Otherwise the process' exit code
+  ##
+  ## On posix, if the process has exited because of a signal, 128 + signal
+  ## number will be returned.
 
 proc inputStream*(p: Process): Stream {.rtl, extern: "nosp$1", tags: [].}
   ## returns ``p``'s input stream for writing to.
@@ -679,6 +686,16 @@ elif not defined(useNimRtl):
     readIdx = 0
     writeIdx = 1
 
+  proc isExitStatus(status: cint): bool =
+    WIFEXITED(status) or WIFSIGNALED(status)
+
+  proc exitStatus(status: cint): cint =
+    if WIFSIGNALED(status):
+      # like the shell!
+      128 + WTERMSIG(status)
+    else:
+      WEXITSTATUS(status)
+
   proc envToCStringArray(t: StringTableRef): cstringArray =
     result = cast[cstringArray](alloc0((t.len + 1) * sizeof(cstring)))
     var i = 0
@@ -967,7 +984,7 @@ elif not defined(useNimRtl):
     var status : cint = 1
     ret = waitpid(p.id, status, WNOHANG)
     if ret == int(p.id):
-      if WIFEXITED(status):
+      if isExitStatus(status):
         p.exitStatus = status
         return false
       else:
@@ -990,7 +1007,9 @@ elif not defined(useNimRtl):
     import kqueue, times
 
     proc waitForExit(p: Process, timeout: int = -1): int =
-      if p.exitStatus != -3: return((p.exitStatus and 0xFF00) shr 8)
+      if p.exitStatus != -3:
+        return exitStatus(p.exitStatus)
+
       if timeout == -1:
         var status : cint = 1
         if waitpid(p.id, status, 0) < 0:
@@ -1041,7 +1060,7 @@ elif not defined(useNimRtl):
         finally:
           discard posix.close(kqFD)
 
-      result = ((p.exitStatus and 0xFF00) shr 8)
+      result = exitStatus(p.exitStatus)
   else:
     import times
 
@@ -1077,7 +1096,9 @@ elif not defined(useNimRtl):
       # ``waitPid`` fails if the process is not running anymore. But then
       # ``running`` probably set ``p.exitStatus`` for us. Since ``p.exitStatus`` is
       # initialized with -3, wrong success exit codes are prevented.
-      if p.exitStatus != -3: return((p.exitStatus and 0xFF00) shr 8)
+      if p.exitStatus != -3:
+        return exitStatus(p.exitStatus)
+
       if timeout == -1:
         var status : cint = 1
         if waitpid(p.id, status, 0) < 0:
@@ -1151,17 +1172,19 @@ elif not defined(useNimRtl):
             if sigprocmask(SIG_UNBLOCK, nmask, omask) == -1:
               raiseOSError(osLastError())
 
-      result = ((p.exitStatus and 0xFF00) shr 8)
+      result = exitStatus(p.exitStatus)
 
   proc peekExitCode(p: Process): int =
     var status = cint(0)
     result = -1
-    if p.exitStatus != -3: return((p.exitStatus and 0xFF00) shr 8)
+    if p.exitStatus != -3:
+      return exitStatus(p.exitStatus)
+
     var ret = waitpid(p.id, status, WNOHANG)
     if ret > 0:
-      if WIFEXITED(status):
+      if isExitStatus(status):
         p.exitStatus = status
-        result = (status and 0xFF00) shr 8
+        result = exitStatus(status)
 
   proc createStream(stream: var Stream, handle: var FileHandle,
                     fileMode: FileMode) =
@@ -1189,7 +1212,8 @@ elif not defined(useNimRtl):
 
   proc execCmd(command: string): int =
     when defined(linux):
-      result = csystem(command) shr 8
+      let tmp = csystem(command)
+      result = if tmp == -1: tmp else: exitStatus(tmp)
     else:
       result = csystem(command)
 
diff --git a/tests/osproc/texitsignal.nim b/tests/osproc/texitsignal.nim
new file mode 100644
index 000000000..c0bed70ee
--- /dev/null
+++ b/tests/osproc/texitsignal.nim
@@ -0,0 +1,36 @@
+discard """
+  output: '''true
+true'''
+  targets: "c"
+"""
+
+import os, osproc
+when not defined(windows):
+  import posix
+
+# Checks that the environment is passed correctly in startProcess
+# To do that launches a copy of itself with a new environment.
+
+if paramCount() == 0:
+  # Parent process
+
+  let p = startProcess(
+    getAppFilename(),
+    args = @["child"],
+    options = {poStdErrToStdOut, poUsePath, poParentStreams}
+  )
+
+  echo p.running()
+
+  p.kill()
+
+  when defined(windows):
+    # windows kill happens using TerminateProcess(h, 0), so we should get a
+    # 0 here
+    echo p.waitForExit() == 0
+  else:
+    # on posix (non-windows), kill sends SIGKILL
+    echo p.waitForExit() == 128 + SIGKILL
+
+else:
+  sleep(5000)  # should get killed before this
\ No newline at end of file
diff --git a/web/news/e031_version_0_16_2.rst b/web/news/e031_version_0_16_2.rst
index fd12bb822..07b67ef0f 100644
--- a/web/news/e031_version_0_16_2.rst
+++ b/web/news/e031_version_0_16_2.rst
@@ -56,6 +56,8 @@ Changes affecting backwards compatibility
   checks. When fields within case objects are initialiazed, the compiler will
   now demand that the respective discriminator field has a matching known
   compile-time value.
+- On posix, the results of `waitForExit`, `peekExitCode`, `execCmd` will return
+  128 + signal number if the application terminates via signal.
 
 Library Additions
 -----------------