On 6/29/26 14:51, Gabriele Monaco wrote:
Please cut down the context a bit more next time, it makes it much
easier to find your review.
On Mon, 2026-06-29 at 01:10 +0800, Wen Yang wrote:
On 6/25/26 20:14, Gabriele Monaco wrote:
+ eval "$TIMEOUT" "$command" &> check_output.$$ &
+ bgpid=$!
+ pid=$(pgrep -f "${command%%[|;&>]*}" | tail -n1)
The pgrep runs may immediately after the background fork, before the
child process has had time to exec.
Yeah I'm aware of this but kind of ignored it for now and never seen it
making troubles in practice..
I could add some delay waiting for the task like:
while [ -z "$pid" ]; do
sleep .5
pid=$(pgrep -f "${command%%[|;&>]*}" | tail -n1)
done
With probably a maximum of some N retrials in case the task never
started or we messed up the pattern.
That may still race in case the command exits before we pgrep it, but in
practice that shouldn't be a problem in our tests.
Any better idea? We cannot really rely on the shell's $! because command
is using a combination of eval+timer and we'd get the wrong pid.
- Since $bgpid is the timeout process, its direct child is exactly the
command we want. Using pgrep -P $bgpid avoids the fragile pattern
matching of pgrep -f and won't accidentally match unrelated
processes with a similar command string, eg:
for i in $(seq 10); do
pid=$(pgrep -P "$bgpid" | head -1)
[ -n "$pid" ] && break
sleep 0.5
done
Note: a bounded retry loop may be necessary; without an upper limit
the loop hangs indefinitely if the command fails to exec.
- For the verbose test specifically ("my pid is $pid"), the pid already
appears in rv's own output. An alternative is to match it with a
numeric pattern instead:
"my pid is [0-9]\+"
This sidesteps the race entirely for that test case.
--
Best wishes,
Wen
