Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fixes #23825; Busy wait on waitid, sleeping at regular intervals #23826

Merged
merged 3 commits into from
Jul 12, 2024
Merged

fixes #23825; Busy wait on waitid, sleeping at regular intervals #23826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fdc6702
Fix #23825; Busy wait on waitid, sleeping in regular intervals
maleyva1 Jul 12, 2024
7c28ffa
Fix (tosproc): Compatibility fix with older tosproc test code
maleyva1 Jul 12, 2024
dc8a969
Fix: C++ codegen generates invalid cast.
maleyva1 Jul 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 42 additions & 101 deletions lib/pure/osproc.nim
View file
Open in desktop
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
Open in desktop
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)
Loading