Skip to content

Commit

Permalink
fixes #23825; Busy wait on waitid, sleeping at regular intervals (#23826
Browse files Browse the repository at this point in the history 
)

Addresses #23825 by using the approaching described in
#23743 (comment).

This takes the approach from Python's `subprocess` library which calls
`waitid` in loop, while sleeping at regular intervals.

CC @alex65536
  • Loading branch information
@maleyva1
maleyva1 authored Jul 12, 2024
1 parent 6d7ab08 commit 58b36bd
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 102 deletions.
143 changes: 42 additions & 101 deletions lib/pure/osproc.nim
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ proc processID*(p: Process): int {.rtl, extern: "nosp$1".} =
return p.id

proc waitForExit*(p: Process, timeout: int = -1): int {.rtl,
extern: "nosp$1", raises: [OSError, ValueError], tags: [].}
extern: "nosp$1", raises: [OSError, ValueError], tags: [TimeEffect].}
## Waits for the process to finish and returns `p`'s error code.
##
## .. warning:: Be careful when using `waitForExit` for processes created without
Expand Down Expand Up @@ -457,7 +457,7 @@ proc execProcesses*(cmds: openArray[string],
if afterRunEvent != nil: afterRunEvent(i, p)
close(p)

iterator lines*(p: Process, keepNewLines = false): string {.since: (1, 3), raises: [OSError, IOError, ValueError], tags: [ReadIOEffect].} =
iterator lines*(p: Process, keepNewLines = false): string {.since: (1, 3), raises: [OSError, IOError, ValueError], tags: [ReadIOEffect, TimeEffect].} =
## Convenience iterator for working with `startProcess` to read data from a
## background process.
##
Expand Down Expand Up @@ -487,7 +487,7 @@ iterator lines*(p: Process, keepNewLines = false): string {.since: (1, 3), raise
discard waitForExit(p)

proc readLines*(p: Process): (seq[string], int) {.since: (1, 3),
raises: [OSError, IOError, ValueError], tags: [ReadIOEffect].} =
raises: [OSError, IOError, ValueError], tags: [ReadIOEffect, TimeEffect].} =
## Convenience function for working with `startProcess` to read data from a
## background process.
##
Expand Down Expand Up @@ -1355,114 +1355,55 @@ elif not defined(useNimRtl):
else:
import std/times

const
hasThreadSupport = compileOption("threads") and not defined(nimscript)

proc waitForExit(p: Process, timeout: int = -1): int =
template adjustTimeout(t, s, e: Timespec) =
var diff: int
var b: Timespec
b.tv_sec = e.tv_sec
b.tv_nsec = e.tv_nsec
e.tv_sec = e.tv_sec - s.tv_sec
if e.tv_nsec >= s.tv_nsec:
e.tv_nsec -= s.tv_nsec
else:
if e.tv_sec == posix.Time(0):
raise newException(ValueError, "System time was modified")
else:
diff = s.tv_nsec - e.tv_nsec
e.tv_nsec = 1_000_000_000 - diff
t.tv_sec = t.tv_sec - e.tv_sec
if t.tv_nsec >= e.tv_nsec:
t.tv_nsec -= e.tv_nsec
else:
t.tv_sec = t.tv_sec - posix.Time(1)
diff = e.tv_nsec - t.tv_nsec
t.tv_nsec = 1_000_000_000 - diff
s.tv_sec = b.tv_sec
s.tv_nsec = b.tv_nsec

if p.exitFlag:
return exitStatusLikeShell(p.exitStatus)

if timeout == -1:
var status: cint = 1
if timeout < 0:
# Backwards compatibility with previous verison to
# handle cases where timeout == -1, but extend
# to handle cases where timeout < 0
var status: cint
if waitpid(p.id, status, 0) < 0:
raiseOSError(osLastError())
p.exitFlag = true
p.exitStatus = status
else:
var nmask, omask: Sigset
var sinfo: SigInfo
var stspec, enspec, tmspec: Timespec

discard sigemptyset(nmask)
discard sigemptyset(omask)
discard sigaddset(nmask, SIGCHLD)

when hasThreadSupport:
if pthread_sigmask(SIG_BLOCK, nmask, omask) == -1:
raiseOSError(osLastError())
else:
if sigprocmask(SIG_BLOCK, nmask, omask) == -1:
raiseOSError(osLastError())

if timeout >= 1000:
tmspec.tv_sec = posix.Time(timeout div 1_000)
tmspec.tv_nsec = (timeout %% 1_000) * 1_000_000
else:
tmspec.tv_sec = posix.Time(0)
tmspec.tv_nsec = (timeout * 1_000_000)

try:
if not running(p):
return exitStatusLikeShell(p.exitStatus)
if clock_gettime(CLOCK_REALTIME, stspec) == -1:
raiseOSError(osLastError())
while true:
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.exitFlag = true
p.exitStatus = status
break
else:
# we have SIGCHLD, but not for process we are waiting,
# so we need to adjust timeout value and continue
if clock_gettime(CLOCK_REALTIME, enspec) == -1:
raiseOSError(osLastError())
adjustTimeout(tmspec, stspec, enspec)
elif res < 0:
let err = osLastError()
if err.cint == EINTR:
# we have received another signal, so we need to
# adjust timeout and continue
if clock_gettime(CLOCK_REALTIME, enspec) == -1:
raiseOSError(osLastError())
adjustTimeout(tmspec, stspec, enspec)
elif err.cint == EAGAIN:
# 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.exitFlag = true
p.exitStatus = status
break
else:
raiseOSError(err)
finally:
when hasThreadSupport:
if pthread_sigmask(SIG_UNBLOCK, nmask, omask) == -1:
raiseOSError(osLastError())
# Max 1ms delay
const maxWait = initDuration(milliseconds = 1)
let wait = initDuration(milliseconds = timeout)
let deadline = getTime() + wait
# starting 50μs delay
var delay = initDuration(microseconds = 50)

while true:
var status: cint
let pid = waitpid(p.id, status, WNOHANG)
if p.id == pid :
p.exitFlag = true
p.exitStatus = status
break
elif pid.int == -1:
raiseOsError(osLastError())
else:
if sigprocmask(SIG_UNBLOCK, nmask, omask) == -1:
raiseOSError(osLastError())
# Continue waiting if needed
if getTime() >= deadline:
# Previous version of `waitForExit`
# foricibly killed the process.
# We keep this so we don't break programs
# that depend on this behavior
if posix.kill(p.id, SIGKILL) < 0:
raiseOSError(osLastError())
else:
let newWait = getTime() + delay
var
waitSpec: TimeSpec
unused: Timespec
waitSpec.tv_sec = posix.Time(newWait.toUnix())
waitSpec.tv_nsec = newWait.nanosecond.clong
discard posix.clock_nanosleep(CLOCK_REALTIME, TIMER_ABSTIME, waitSpec, unused)
let remaining = deadline - getTime()
delay = min([delay * 2, remaining, maxWait])

result = exitStatusLikeShell(p.exitStatus)

Expand Down
17 changes: 16 additions & 1 deletion tests/osproc/twaitforexit.nim
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,19 @@ block: # bug #5091
discard

# check that we don't have to wait msWait milliseconds
doAssert(getTime() < atStart + milliseconds(msWait))
doAssert(getTime() < atStart + milliseconds(msWait))

block: # bug #23825
var thr: array[0..99, Thread[int]]

proc threadFunc(i: int) {.thread.} =
let sleepTime = float(i) / float(thr.len + 1)
doAssert sleepTime < 1.0
let p = startProcess("sleep", workingDir = "", args = @[$sleepTime], options = {poUsePath, poParentStreams})
# timeout = 1_000_000 seconds ~= 278 hours ~= 11.5 days
doAssert p.waitForExit(timeout=1_000_000_000) == 0

for i in low(thr)..high(thr):
createThread(thr[i], threadFunc, i)

joinThreads(thr)

0 comments on commit 58b36bd

Please sign in to comment.