Remove unneeded safe_sleep

[vtrenton]

Hey Github Runner team!

I felt the need to open this PR as to at least have a discussion around this. safe_sleep has created serious problems in the past with wasting CPU cycles using #3792 as reference. The codebase keeps getting bigger with hacks like using ping as a timer... Which is great except it relies heavily on the assumption that ping on all systems will never change it send frequency (unlikely - but there is a potential).

There is also an existing concern about the latest fix being vulnerable to injection attacks. Although not likely exploitable - it's new risk introduced into the codebase: #3792 (comment)

There is no need for this as almost zero UNIX-Like systems including containers and Android/Embedded systems don't have sleep. Sleep is a standard POSIX utility on all UNIX systems.

To top it off these scripts require BASH which is installed on less systems than sleep. Let alone hardcoding the shebang path to /bin/bash which systems such as Nix and BSD do not use. BASH is much larger compatibility risk than sleep.

if you're making bash (a much less deployed tool) a hard requirement than sleep should be.

All of this is to say - we should remove safe_sleep from the codebase and just use/assume a POSIX sleep implementation is available. This is technical debt and the implementation costs much more than its worth.

I would like to challenge anyone to give me an example of a UNIX-Like system that uses BASH that doesn't have sleep. Very open to being challenged on this. I seriously for the life of me can think of a single one. Would love to hear any feedback on this!

Thank you for your consideration!

[Elyytscha]

At the very least, maybe safe_sleep should prefer the system sleep binary when it exists (I think thats how it was designed initially), and only fall back to this bespoke shell-loop masterpiece when it doesn’t.

Much easier to meter runner minutes when your sleep actually wakes up

[vtrenton]

At the very least, maybe safe_sleep should prefer the system sleep binary when it exists (I think thats how it was designed initially), and only fall back to this bespoke shell-loop masterpiece when it doesn’t.

Much easier to meter runner minutes when your sleep actually wakes up

PRs have since been merged to reimplement this approach:

runner/src/Misc/layoutroot/safe_sleep.sh

Line 3 in c96dcd4

# try to use sleep if available

I agree it's much better than just the while loop. But my intention of this PR is to remove safe_sleep altogether. As I feel this is overengineering for a problem that doesn't exist. This design has created much more of a problem than it's worth.

If we want to go down the compatibility rabbit hole - we can. Does bash exist? does command exist? should we write a safe_bash and safe_command? when does it stop.