tree e233291cddf5908688f36f344dd6bfa59ec448a7
parent fe8d89def25dd836791c57c839a66619f532ab62
author Brian Norris <briannorris@chromium.org> 1712355650 -0700
committer Chromeos LUCI <chromeos-scoped@luci-project-accounts.iam.gserviceaccount.com> 1714180221 +0000

CHROMIUM: test_util_common: Resolve timed_waitpid() races

timed_waitpid() claims to be simply a version of waitpid() with a
timeout, but it has races that prevent it from being reliable.

In particular, we're setting up a pipe and a SIGCHLD handler on the
first time through. This handler may be registered *after* the PID we're
looking for has died (i.e., race condition), and so that handler may
never write to the pipe that we're waiting (select()) on.

Additionally, if we somehow manage to pass the select() too early (e.g.,
SIGCHLD for the wrong PID?), then we enter a loop where we no longer
respect the timeout -- if our expected child PID never changes state,
we'll loop forever.

Rewrite this implementation to:
1. never use msleep()
2. always check waitpid() first, in case the process in question is dead
   before we open the SIGCHLD handler
3. reuse the |tv| timeout for multiple select() calls -- this is
   Linux-specific behavior, that select() will update the |tv| to
   contain the remaining time.

This resolves test failures seen like the following:

...with child exit notification before child setup success notification
BAD: wrong value for timed_waitpid (pid, 5), expected 2510 got 0
        at tests/test_job_process.c:8969 (test_handler).
FAIL test_job_process (exit status: 134)

(They're especially noticeable if we inject extra delay immediately
before the timed_waitpid() call.)

Because this is no longer an infinite wait, there are also cases where
we need to increase the timeout. Significantly increase the QUIESCE_*
waits, because these really aren't guarantees we can keep, given a
loaded enough test system.

BUG=b:332653406, b:232122437
TEST=`FEATURES=test emerge-${BOARD} upstart`

Change-Id: I4bb5987ceca385a841154f6345850f3e6c24758f
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/upstart/+/5431197
Reviewed-by: Allen Webb <allenwebb@google.com>
Tested-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
Commit-Queue: Brian Norris <briannorris@chromium.org>