[igt-dev] [PATCH i-g-t] lib: Kill residual children at the end of a subtest

[Chris Wilson]

Quoting Petri Latvala (2020-02-03 11:50:07)
> On Mon, Feb 03, 2020 at 11:21:56AM +0000, Chris Wilson wrote:
> > Quoting Petri Latvala (2020-02-03 11:13:44)
> > > On Sun, Feb 02, 2020 at 06:56:49PM +0000, Chris Wilson wrote:
> > > > Ensure that we tidy up all the excess children left behind by a failing
> > > > subtest, we do not want them loitering into the next!
> > > > 
> > > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > > ---
> > > >  lib/igt_core.c | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > > 
> > > > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > > > index a0bf29408..65c80fbba 100644
> > > > --- a/lib/igt_core.c
> > > > +++ b/lib/igt_core.c
> > > > @@ -1362,6 +1362,10 @@ static void exit_subtest(const char *result)
> > > >  
> > > >       igt_terminate_spins();
> > > >  
> > > > +     for (int c = 0; c < num_test_children; c++)
> > > > +             kill(test_children[c], SIGKILL);
> > > > +     num_test_children = 0;
> > > 
> > > Are the children wait()ed for somewhere?
> > 
> > Would be very hard to do from inside the dead subtest. We can leave them
> > as zombies, otherwise how long do you want to wait? As far as the CPU
> > side of things go, they will not make any further progress.
> 
> igt_exit() does an assert that waitpid(-1, ...) errors with
> ECHILD. The exit handler that does call wait() only does it if
> num_test_children is nonzero.

Hah. That doesn't work :) Since it doesn't wait, the children aren't
dead by that point.

> All tests that fork stuff are already supposed to call
> igt_waitchildren(), right? Should we call igt_waitchildren_timeout()
> here instead, with a 1s timeout? Or __igt_waitchildren() after firing
> off some SIGKILLs.

The test does call wait, but it has failed before it can do so. If only
we had some subtest closures :)
-Chris