From 5986b78bbef2dd81a1430a1a41668f1d3b5fd1f8 Mon Sep 17 00:00:00 2001 From: Johannes Hofmann Date: Thu, 22 Sep 2016 10:55:21 +0200 Subject: [PATCH 1/7] generally update exitCode only after successful completion of waitpid() --- lib/pure/osproc.nim | 49 ++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/lib/pure/osproc.nim b/lib/pure/osproc.nim index abc21b2b2..4003882e6 100644 --- a/lib/pure/osproc.nim +++ b/lib/pure/osproc.nim @@ -957,13 +957,10 @@ elif not defined(useNimRtl): proc running(p: Process): bool = var ret : int - when not defined(freebsd): - ret = waitpid(p.id, p.exitCode, WNOHANG) - else: - var status : cint = 1 - ret = waitpid(p.id, status, WNOHANG) - if WIFEXITED(status): - p.exitCode = status + var status : cint = 1 + ret = waitpid(p.id, status, WNOHANG) + if WIFEXITED(status): + p.exitCode = status if ret == 0: return true # Can't establish status. Assume running. result = ret == int(p.id) @@ -982,9 +979,10 @@ elif not defined(useNimRtl): proc waitForExit(p: Process, timeout: int = -1): int = if p.exitCode != -3: return p.exitCode if timeout == -1: - if waitpid(p.id, p.exitCode, 0) < 0: - p.exitCode = -3 + var status : cint = 1 + if waitpid(p.id, status, 0) < 0: raiseOSError(osLastError()) + p.exitCode = status else: var kqFD = kqueue() if kqFD == -1: @@ -1004,6 +1002,7 @@ elif not defined(useNimRtl): try: while true: + var status : cint = 1 var count = kevent(kqFD, addr(kevIn), 1, addr(kevOut), 1, addr(tmspec)) if count < 0: @@ -1014,15 +1013,15 @@ elif not defined(useNimRtl): # timeout expired, so we trying to kill process if posix.kill(p.id, SIGKILL) == -1: raiseOSError(osLastError()) - if waitpid(p.id, p.exitCode, 0) < 0: - p.exitCode = -3 + if waitpid(p.id, status, 0) < 0: raiseOSError(osLastError()) + p.exitCode = status break else: if kevOut.ident == p.id.uint and kevOut.filter == EVFILT_PROC: - if waitpid(p.id, p.exitCode, 0) < 0: - p.exitCode = -3 + if waitpid(p.id, status, 0) < 0: raiseOSError(osLastError()) + p.exitCode = status break else: raiseOSError(osLastError()) @@ -1036,6 +1035,8 @@ elif not defined(useNimRtl): const hasThreadSupport = compileOption("threads") and not defined(nimscript) + var status : cint = 1 + proc waitForExit(p: Process, timeout: int = -1): int = template adjustTimeout(t, s, e: Timespec) = var diff: int @@ -1067,9 +1068,9 @@ elif not defined(useNimRtl): # initialized with -3, wrong success exit codes are prevented. if p.exitCode != -3: return p.exitCode if timeout == -1: - if waitpid(p.id, p.exitCode, 0) < 0: - p.exitCode = -3 + if waitpid(p.id, status, 0) < 0: raiseOSError(osLastError()) + p.exitCode = status else: var nmask, omask: Sigset var sinfo: SigInfo @@ -1100,9 +1101,9 @@ elif not defined(useNimRtl): let res = sigtimedwait(nmask, sinfo, tmspec) if res == SIGCHLD: if sinfo.si_pid == p.id: - if waitpid(p.id, p.exitCode, 0) < 0: - p.exitCode = -3 + if waitpid(p.id, status, 0) < 0: raiseOSError(osLastError()) + p.exitCode = status break else: # we have SIGCHLD, but not for process we are waiting, @@ -1122,9 +1123,9 @@ elif not defined(useNimRtl): # timeout expired, so we trying to kill process if posix.kill(p.id, SIGKILL) == -1: raiseOSError(osLastError()) - if waitpid(p.id, p.exitCode, 0) < 0: - p.exitCode = -3 + if waitpid(p.id, status, 0) < 0: raiseOSError(osLastError()) + p.exitCode = status break else: raiseOSError(err) @@ -1139,14 +1140,16 @@ elif not defined(useNimRtl): result = int(p.exitCode) shr 8 proc peekExitCode(p: Process): int = + var status : cint = 1 if p.exitCode != -3: return p.exitCode - var ret = waitpid(p.id, p.exitCode, WNOHANG) + var ret = waitpid(p.id, status, WNOHANG) var b = ret == int(p.id) if b: result = -1 - if not WIFEXITED(p.exitCode): - p.exitCode = -3 + if WIFEXITED(status): + p.exitCode = status + result = p.exitCode.int shr 8 + else: result = -1 - else: result = p.exitCode.int shr 8 proc createStream(stream: var Stream, handle: var FileHandle, fileMode: FileMode) = From 829b70644069d2ce6760ad4c31d598722c282418 Mon Sep 17 00:00:00 2001 From: Johannes Hofmann Date: Sat, 24 Sep 2016 14:46:07 +0200 Subject: [PATCH 2/7] rename exitCode to exitStatus --- lib/pure/osproc.nim | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/lib/pure/osproc.nim b/lib/pure/osproc.nim index 4003882e6..ac8fa7a14 100644 --- a/lib/pure/osproc.nim +++ b/lib/pure/osproc.nim @@ -48,7 +48,7 @@ type inHandle, outHandle, errHandle: FileHandle inStream, outStream, errStream: Stream id: Pid - exitCode: cint + exitStatus: cint options: set[ProcessOption] Process* = ref ProcessObj ## represents an operating system process @@ -731,7 +731,7 @@ elif not defined(useNimRtl): pStdin, pStdout, pStderr: array[0..1, cint] new(result) result.options = options - result.exitCode = -3 # for ``waitForExit`` + result.exitStatus = -3 # for ``waitForExit`` if poParentStreams notin options: if pipe(pStdin) != 0'i32 or pipe(pStdout) != 0'i32 or pipe(pStderr) != 0'i32: @@ -960,7 +960,7 @@ elif not defined(useNimRtl): var status : cint = 1 ret = waitpid(p.id, status, WNOHANG) if WIFEXITED(status): - p.exitCode = status + p.exitStatus = status if ret == 0: return true # Can't establish status. Assume running. result = ret == int(p.id) @@ -977,12 +977,12 @@ elif not defined(useNimRtl): import kqueue, times proc waitForExit(p: Process, timeout: int = -1): int = - if p.exitCode != -3: return p.exitCode + if p.exitStatus != -3: return p.exitStatus if timeout == -1: var status : cint = 1 if waitpid(p.id, status, 0) < 0: raiseOSError(osLastError()) - p.exitCode = status + p.exitStatus = status else: var kqFD = kqueue() if kqFD == -1: @@ -1015,20 +1015,20 @@ elif not defined(useNimRtl): raiseOSError(osLastError()) if waitpid(p.id, status, 0) < 0: raiseOSError(osLastError()) - p.exitCode = status + p.exitStatus = status break else: if kevOut.ident == p.id.uint and kevOut.filter == EVFILT_PROC: if waitpid(p.id, status, 0) < 0: raiseOSError(osLastError()) - p.exitCode = status + p.exitStatus = status break else: raiseOSError(osLastError()) finally: discard posix.close(kqFD) - result = int(p.exitCode) shr 8 + result = int(p.exitStatus) shr 8 else: import times @@ -1062,15 +1062,15 @@ elif not defined(useNimRtl): s.tv_sec = b.tv_sec s.tv_nsec = b.tv_nsec - #if waitPid(p.id, p.exitCode, 0) == int(p.id): + #if waitPid(p.id, p.exitStatus, 0) == int(p.id): # ``waitPid`` fails if the process is not running anymore. But then - # ``running`` probably set ``p.exitCode`` for us. Since ``p.exitCode`` is + # ``running`` probably set ``p.exitStatus`` for us. Since ``p.exitStatus`` is # initialized with -3, wrong success exit codes are prevented. - if p.exitCode != -3: return p.exitCode + if p.exitStatus != -3: return p.exitStatus if timeout == -1: if waitpid(p.id, status, 0) < 0: raiseOSError(osLastError()) - p.exitCode = status + p.exitStatus = status else: var nmask, omask: Sigset var sinfo: SigInfo @@ -1103,7 +1103,7 @@ elif not defined(useNimRtl): if sinfo.si_pid == p.id: if waitpid(p.id, status, 0) < 0: raiseOSError(osLastError()) - p.exitCode = status + p.exitStatus = status break else: # we have SIGCHLD, but not for process we are waiting, @@ -1125,7 +1125,7 @@ elif not defined(useNimRtl): raiseOSError(osLastError()) if waitpid(p.id, status, 0) < 0: raiseOSError(osLastError()) - p.exitCode = status + p.exitStatus = status break else: raiseOSError(err) @@ -1137,17 +1137,17 @@ elif not defined(useNimRtl): if sigprocmask(SIG_UNBLOCK, nmask, omask) == -1: raiseOSError(osLastError()) - result = int(p.exitCode) shr 8 + result = int(p.exitStatus) shr 8 proc peekExitCode(p: Process): int = var status : cint = 1 - if p.exitCode != -3: return p.exitCode + if p.exitStatus != -3: return p.exitStatus var ret = waitpid(p.id, status, WNOHANG) var b = ret == int(p.id) if b: result = -1 if WIFEXITED(status): - p.exitCode = status - result = p.exitCode.int shr 8 + p.exitStatus = status + result = p.exitStatus.int shr 8 else: result = -1 From 14f72bcbac4ea5c9537aad4791681cea6f86dcc7 Mon Sep 17 00:00:00 2001 From: Johannes Hofmann Date: Sun, 25 Sep 2016 09:51:23 +0200 Subject: [PATCH 3/7] make status variable local --- lib/pure/osproc.nim | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/pure/osproc.nim b/lib/pure/osproc.nim index ac8fa7a14..bd0f122f3 100644 --- a/lib/pure/osproc.nim +++ b/lib/pure/osproc.nim @@ -1035,12 +1035,11 @@ elif not defined(useNimRtl): const hasThreadSupport = compileOption("threads") and not defined(nimscript) - var status : cint = 1 - proc waitForExit(p: Process, timeout: int = -1): int = template adjustTimeout(t, s, e: Timespec) = var diff: int var b: Timespec + var status : cint = 1 b.tv_sec = e.tv_sec b.tv_nsec = e.tv_nsec e.tv_sec = (e.tv_sec - s.tv_sec).Time From 1dccbaf9a0cc0256701cfbae5bbb17b4d4e9416a Mon Sep 17 00:00:00 2001 From: Johannes Hofmann Date: Sun, 25 Sep 2016 10:16:14 +0200 Subject: [PATCH 4/7] another attempt at properly declaring the status variable --- lib/pure/osproc.nim | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/pure/osproc.nim b/lib/pure/osproc.nim index bd0f122f3..be59de120 100644 --- a/lib/pure/osproc.nim +++ b/lib/pure/osproc.nim @@ -1039,7 +1039,6 @@ elif not defined(useNimRtl): template adjustTimeout(t, s, e: Timespec) = var diff: int var b: Timespec - var status : cint = 1 b.tv_sec = e.tv_sec b.tv_nsec = e.tv_nsec e.tv_sec = (e.tv_sec - s.tv_sec).Time @@ -1067,6 +1066,7 @@ elif not defined(useNimRtl): # initialized with -3, wrong success exit codes are prevented. if p.exitStatus != -3: return p.exitStatus if timeout == -1: + var status : cint = 1 if waitpid(p.id, status, 0) < 0: raiseOSError(osLastError()) p.exitStatus = status @@ -1100,6 +1100,7 @@ elif not defined(useNimRtl): let res = sigtimedwait(nmask, sinfo, tmspec) if res == SIGCHLD: if sinfo.si_pid == p.id: + var status : cint = 1 if waitpid(p.id, status, 0) < 0: raiseOSError(osLastError()) p.exitStatus = status @@ -1122,6 +1123,7 @@ elif not defined(useNimRtl): # timeout expired, so we trying to kill process if posix.kill(p.id, SIGKILL) == -1: raiseOSError(osLastError()) + var status : cint = 1 if waitpid(p.id, status, 0) < 0: raiseOSError(osLastError()) p.exitStatus = status From 52db21bb2cb03b498b0cb3b34a49213449b34b2e Mon Sep 17 00:00:00 2001 From: Johannes Hofmann Date: Fri, 30 Sep 2016 10:19:57 +0200 Subject: [PATCH 5/7] convert exitStatus to exit code --- lib/pure/osproc.nim | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/pure/osproc.nim b/lib/pure/osproc.nim index be59de120..6c2debb1b 100644 --- a/lib/pure/osproc.nim +++ b/lib/pure/osproc.nim @@ -977,7 +977,7 @@ elif not defined(useNimRtl): import kqueue, times proc waitForExit(p: Process, timeout: int = -1): int = - if p.exitStatus != -3: return p.exitStatus + if p.exitStatus != -3: return int(p.exitStatus) shr 8 if timeout == -1: var status : cint = 1 if waitpid(p.id, status, 0) < 0: @@ -1064,7 +1064,7 @@ 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 + if p.exitStatus != -3: return int(p.exitStatus) shr 8 if timeout == -1: var status : cint = 1 if waitpid(p.id, status, 0) < 0: @@ -1142,7 +1142,7 @@ elif not defined(useNimRtl): proc peekExitCode(p: Process): int = var status : cint = 1 - if p.exitStatus != -3: return p.exitStatus + if p.exitStatus != -3: return int(p.exitStatus) shr 8 var ret = waitpid(p.id, status, WNOHANG) var b = ret == int(p.id) if b: result = -1 From 8d85809d626e7445889c9f398682147cc5544950 Mon Sep 17 00:00:00 2001 From: Johannes Hofmann Date: Fri, 30 Sep 2016 10:27:59 +0200 Subject: [PATCH 6/7] add testcase for exit code handling --- tests/osproc/texitcode.nim | 18 ++++++++++++++++++ tests/osproc/tfalse.nim | 2 ++ 2 files changed, 20 insertions(+) create mode 100644 tests/osproc/texitcode.nim create mode 100644 tests/osproc/tfalse.nim diff --git a/tests/osproc/texitcode.nim b/tests/osproc/texitcode.nim new file mode 100644 index 000000000..df1db8aa3 --- /dev/null +++ b/tests/osproc/texitcode.nim @@ -0,0 +1,18 @@ +discard """ + file: "texitcode.nim" + output: "" +""" +import osproc, os + +const filename = when defined(Windows): "tfalse.exe" else: "tfalse" + +doAssert fileExists(getCurrentDir() / "tests" / "osproc" / filename) + +var p = startProcess(filename, getCurrentDir() / "tests" / "osproc") +doAssert(waitForExit(p) == QuitFailure) + +p = startProcess(filename, getCurrentDir() / "tests" / "osproc") +var running = true +while running: + running = running(p) +doAssert(waitForExit(p) == QuitFailure) diff --git a/tests/osproc/tfalse.nim b/tests/osproc/tfalse.nim new file mode 100644 index 000000000..a2c5e259d --- /dev/null +++ b/tests/osproc/tfalse.nim @@ -0,0 +1,2 @@ +import system +quit(QuitFailure) From 7f25db2dd1dfbcb22b0a432efd56b522272eeede Mon Sep 17 00:00:00 2001 From: Johannes Hofmann Date: Fri, 30 Sep 2016 10:39:57 +0200 Subject: [PATCH 7/7] rename tfalse.nim to tafalse.nim --- tests/osproc/tafalse.nim | 3 +++ tests/osproc/texitcode.nim | 10 +++++----- tests/osproc/tfalse.nim | 2 -- 3 files changed, 8 insertions(+), 7 deletions(-) create mode 100644 tests/osproc/tafalse.nim delete mode 100644 tests/osproc/tfalse.nim diff --git a/tests/osproc/tafalse.nim b/tests/osproc/tafalse.nim new file mode 100644 index 000000000..24fd4fb2e --- /dev/null +++ b/tests/osproc/tafalse.nim @@ -0,0 +1,3 @@ +# 'tafalse.nim' to ensure it is compiled before texitcode.nim +import system +quit(QuitFailure) diff --git a/tests/osproc/texitcode.nim b/tests/osproc/texitcode.nim index df1db8aa3..1e83658c2 100644 --- a/tests/osproc/texitcode.nim +++ b/tests/osproc/texitcode.nim @@ -4,14 +4,14 @@ discard """ """ import osproc, os -const filename = when defined(Windows): "tfalse.exe" else: "tfalse" +const filename = when defined(Windows): "tafalse.exe" else: "tafalse" +let dir = getCurrentDir() / "tests" / "osproc" +doAssert fileExists(dir / filename) -doAssert fileExists(getCurrentDir() / "tests" / "osproc" / filename) - -var p = startProcess(filename, getCurrentDir() / "tests" / "osproc") +var p = startProcess(filename, dir) doAssert(waitForExit(p) == QuitFailure) -p = startProcess(filename, getCurrentDir() / "tests" / "osproc") +p = startProcess(filename, dir) var running = true while running: running = running(p) diff --git a/tests/osproc/tfalse.nim b/tests/osproc/tfalse.nim deleted file mode 100644 index a2c5e259d..000000000 --- a/tests/osproc/tfalse.nim +++ /dev/null @@ -1,2 +0,0 @@ -import system -quit(QuitFailure)