uutils / uutils/coreutils

what's up waitpid + sleep loop in wait_or_timeout

Open
#6,363 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

U - timeout
Dominant language
Rust
Stars
24.1k
Forks
2k
Avg merge
1d 5h
Merged PRs (30d)
365

Description

I was stracing the timeout replacement and found it spams wait4 and clock_nanosleep calls.

Problematic code is:


    fn wait_or_timeout(&mut self, timeout: Duration) -> io::Result<Option<ExitStatus>> {
        if timeout == Duration::from_micros(0) {
            return self.wait().map(Some);
        }
        // .try_wait() doesn't drop stdin, so we do it manually
        drop(self.stdin.take());

        let start = Instant::now();
        loop {
            if let Some(status) = self.try_wait()? {
                return Ok(Some(status));
            }

            if start.elapsed() >= timeout {
                break;
            }

            // XXX: this is kinda gross, but it's cleaner than starting a thread just to wait
            //      (which was the previous solution).  We might want to use a different duration
            //      here as well
            thread::sleep(Duration::from_millis(100));
        }

        Ok(None)
    }

The original C variant gets away without doing anything of the sort.

Now, I'm not particularly good with rust but would be genuinely surprised if this was really necessary.

One clean solution would be to grab a file descriptor for the child (see the CLONE_PIDFD flag to clone) and one for timerfd. Then the code could nicely wait in an event loop.

I would implement this myself but I'm atrocious at Rust.

If stuff like the above kind be used, what's the reasoning?

While the comment above acknowledges the current state is not good, it does not justify it.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Locate the Rust wait_or_timeout implementation and compare its try_wait plus thread::sleep loop with the original C variant. Investigate the cross-platform process-waiting constraints and existing timeout behavior; done means the rationale is documented or the waiting approach is improved without changing command results or timeout semantics.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
cli, operating-systems, performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.