timeout: replace 100ms polling loop with setitimer for precise timeouts#12352
Open
gabrielhnf wants to merge 1 commit into
Open
timeout: replace 100ms polling loop with setitimer for precise timeouts#12352gabrielhnf wants to merge 1 commit into
gabrielhnf wants to merge 1 commit into
Conversation
|
GNU testsuite comparison: |
a476264 to
251444d
Compare
Author
|
The test_mkfifo failures appear to be unrelated to this PR, can someone confirm? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #11615 and #9099
Problem
uu timeoutwas less precise than GNU timeout by design. The core of the issue was inwait_or_timeoutinprocess.rs, which used a polling loop sleeping in 100ms increments:This meant that for any timeout shorter than 100ms, or any timeout whose deadline fell between two polling ticks,
uu timeoutwould overshoot by up to 100ms. For latency-sensitive use cases this was a meaningful regression from GNU.Before:
After:
Solution
On Unix, replace the polling loop with
setitimer(ITIMER_REAL, ...), which deliversSIGALRMto the process at the specified time with microsecond precision. This is the same mechanism GNU timeout uses (GNU preferstimer_create+timer_settimefor nanosecond precision, falling back tosetitimer; see NOTES below).Design overview
The implementation adds three new functions and a supporting enum:
arm_timer(duration) -> TimerHandleArms the itimer for the given duration. Returns
TimerHandle::Posixon success orTimerHandle::Pollingifsetitimerfails, allowing graceful fallback to the original polling path. Sub-microsecond durations are clamped to 1 microsecond, since a zeroitimervaldisarms the timer rather than firing it. A duration of exactlyDuration::ZERObypasses the itimer entirely and falls through to polling, which correctly handles the "no timeout" semantics.disarm_timer()Disarms the itimer by writing a zeroed
itimerval. Called afterwaitpidreturns so the timer cannot fire during later execution.wait_with_itimer(process, foreground) -> io::Result<Option<ExitStatus>>Replaces the
wait_or_timeoutcall in the main wait path. Callslibc::waitpiddirectly rather than going throughChild::wait, becauseChild::waitwrapswaitpidin anEINTR-retry loop, which would silently swallow theEINTRwe need to observe when SIGALRM fires.The loop distinguishes three cases on
EINTR:SIGNALED && RECEIVED_SIGNAL == SIGALRM: the timer fired. Disarm andreturn
Ok(None)— child is still alive, caller sendsterm_signal.SIGNALED && RECEIVED_SIGNAL != SIGALRM: an external signal (e.g.SIGINT, SIGTERM) was delivered to the timeout process. Forward it to the
child via
send_signal, reset the flag, and retrywaitpid.EINTRfrom some other source, retrywaitpid.A pre-loop check handles the race where the timer fires before
waitpidis even called (relevant for very short durations after clamping to 1µs).Signal handler changes
All signal handlers are now registered with
sigactionusingSaFlags::empty(), intentionally omittingSA_RESTART, which was being implicitly set bynix::signal. WithoutSA_RESTART,waitpidreturnsEINTRwhen any signal arrives, giving the loop a chance to inspectRECEIVED_SIGNALand act accordingly.External signal forwarding
When an external signal interrupts
waitpid,wait_with_itimerforwards it to the child immediately using the existingsend_signalfunction, which correctly handles theforegroundflag.--kill-afterpathwait_or_kill_processis updated to usewait_with_itimeron the sameTimerHandle::Posixpath, so--kill-afteralso benefits from itimer precision. TheSIGNALEDandRECEIVED_SIGNALatomics are reset before entering thekill_afterphase so stale state from the first timeout does not corrupt the second wait.Notes
timer_create/timer_settimenot yet used.GNU timeout prefers
timer_create(CLOCK_REALTIME)for nanosecond resolution. This PR stops atsetitimer(microsecond precision) mainly due to portability, since some UNIX don't implementtimer_create. A follow up can addtimer_createas the preferred path withsetitimeras fallback, matching GNU's priority order exactly.