Ensure safe_sleep tries alternative approaches#4146
Conversation
… CPU-intensive busy-waiting.
There was a problem hiding this comment.
Pull request overview
This PR enhances the safe_sleep.sh script by implementing a cascading fallback strategy for sleep functionality. Instead of immediately falling back to a CPU-intensive busy-wait loop, the script now attempts multiple alternative approaches in order of preference.
Key Changes:
- Adds primary check for the standard
sleepcommand before attempting alternatives - Implements a
ping-based sleep as a secondary fallback - Adds a
read -ttimeout-based sleep for bash environments as a tertiary option - Retains the existing busy-wait loop as the final fallback
Comments suppressed due to low confidence (2)
src/Misc/layoutroot/safe_sleep.sh:5
- The parameter
$1is not validated before use. If the script is called without arguments or with invalid input (non-numeric, negative numbers, or special characters), it could cause unexpected behavior or errors in the various sleep implementations. Consider adding input validation at the beginning of the script to ensure$1is a valid positive number.
sleep "$1"
src/Misc/layoutroot/safe_sleep.sh:37
- The busy-wait fallback continuously checks the SECONDS variable in a tight loop, which will consume 100% CPU for the duration of the sleep. This is inefficient and can impact system performance, especially for longer sleep durations. While this is a fallback for when no other option is available, consider adding a comment to warn about this behavior, or implement a less aggressive polling mechanism if possible.
SECONDS=0
while [[ $SECONDS -lt $1 ]]; do
:
done
|
This isn't TTY-agnostic, because the Also, Here's a more robust solution: #!/usr/bin/env bash
secs=${1:-0}
if command -v sleep >/dev/null 2>&1; then
sleep "$secs"
exit 0
fi
if [ -n "${BASH_VERSION-}" ]; then
# Bash >= 4 has coproc, which gives us a TTY-agnostic approach using pure bash builtins.
if [ "${BASH_VERSINFO[0]}" -ge 4 ] 2>/dev/null; then
coproc _slp { read; }
read -t "$secs" <&"${_slp[0]}" || :
kill "$_slp_PID" 2>/dev/null || :
wait "$_slp_PID" 2>/dev/null || :
exit 0
fi
# Creates a PTY with tail instead.
if command -v tail >/dev/null 2>&1; then
exec 3< <(tail -f /dev/null)
read -t "$secs" <&3 || :
exec 3<&-
exit 0
fi
# If we have a controlling TTY.
if exec 3</dev/tty 2>/dev/null; then
read -t "$secs" <&3 || :
exec 3<&-
exit 0
fi
fi
if command -v ping >/dev/null 2>&1; then
ping -c "$((secs + 1))" 127.0.0.1 >/dev/null 2>&1 || :
exit 0
fi
SECONDS=0
while (( SECONDS < secs )); do
:
doneThere are actually a bunch of |
I mean you could submit another PR |

Special thanks to everyone for their contributions and ideas in resolving this issue:
sleep#4138)