Re: [igt-dev] [PATCH i-g-t] lib: Make sure we leak no child processes

Chris Wilson
2019-02-12T16:48:45Z

Quoting Daniel Vetter (2019-02-11 22:43:15)
> There's a lot more ways to leak children than igt_fork, some even
> handrolled. So check for that. Also have a nice littel testcase for
> that too.
>
> v2: Don't hang if there's a leaked child process (Chris). Has the
> added benefit that my library unit test also gets faster!
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

> ---
> lib/igt_core.c | 4 ++++
> lib/tests/igt_fork.c | 18 ++++++++++++++++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 3053697da58c..d2cfc8e6da20 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -1492,6 +1492,8 @@ void __igt_fail_assert(const char *domain, const char *file, const int line,
> */
> void igt_exit(void)
> {
> + int tmp;
> +
> igt_exit_called = true;
>
> if (igt_key_file)
> @@ -1527,6 +1529,8 @@ void igt_exit(void)
> kill(test_children[c], SIGKILL);
> assert(!num_test_children);
>
> + assert(waitpid(-1, &tmp, WNOHANG) == -1 && errno == ECHILD);
> +
> if (!test_with_subtests) {
> struct timespec now;
> const char *result;
> diff --git a/lib/tests/igt_fork.c b/lib/tests/igt_fork.c
> index e5b0ab016b23..b486d07000bb 100644
> --- a/lib/tests/igt_fork.c
> +++ b/lib/tests/igt_fork.c
> @@ -68,6 +68,20 @@ static void igt_fork_leak(void)
> }
> }
>
> +static void plain_fork_leak(void)
> +{
> + int pid;
> +
> + switch (pid = fork()) {
> + case -1:
> + internal_assert(0);
> + case 0:
> + sleep(1);
> + default:
> + exit(0);
> + }
> +}
> +
> static void igt_fork_timeout_leak(void)
> {
> igt_fork(i, 1) {
> @@ -122,4 +136,8 @@ int main(int argc, char **argv)
> /* check that igt_waitchildren_timeout cleans up*/
> ret = do_fork(igt_fork_timeout_leak);
> internal_assert(ret == IGT_EXIT_FAILURE);
> +
> + /* check that any other process leaks are caught*/
> + ret = do_fork(plain_fork_leak);
> + internal_assert(ret == SIGABRT + 128);

I'm mulling over
internal_assert(signaled(status, SIGABRT));

static bool signaled(int status, int sig)
{
return WIFSIGNALED(status) && WTERMSIG(status) == sig;
}

static bool exited(int status, int code)
{
return WIFEXITED(status) && WEXITSTATUS(status) == code;
}
-Chris