Re: [OE-core] [PATCH 3/3] testimage.bbclass: capture RuntimeError too

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Mon, 2024-11-18 at 10:00 +0200, Mikko Rapeli wrote:
> On Tue, Nov 12, 2024 at 11:25:51AM +0000, Richard Purdie wrote:
> > On Mon, 2024-11-11 at 13:16 +0000, Mikko Rapeli via lists.openembedded.org wrote:
> > > runqemu can fail with RuntimeError exception. Non-cought exception
> > > causes cooker process leaks which bind to successive bitbake command
> > > line calls and that can cause really odd errors to users, e.g. when
> > > build/tmp is wiped and cooker processes expect files to be there.
> > > 
> > > Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
> > > ---
> > >  meta/classes-recipe/testimage.bbclass | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/meta/classes-recipe/testimage.bbclass b/meta/classes-recipe/testimage.bbclass
> > > index 19075ce1f3..a9b031093a 100644
> > > --- a/meta/classes-recipe/testimage.bbclass
> > > +++ b/meta/classes-recipe/testimage.bbclass
> > > @@ -371,7 +371,7 @@ def testimage_main(d):
> > >          complete = True
> > >          if results.hasAnyFailingTest():
> > >              run_failed_tests_post_actions(d, tc)
> > > -    except (KeyboardInterrupt, BlockingIOError) as err:
> > > +    except (KeyboardInterrupt, BlockingIOError, RuntimeError) as err:
> > >          if isinstance(err, KeyboardInterrupt):
> > >              bb.error('testimage interrupted, shutting down...')
> > >          else:
> > > 
> > 
> > During review it is hard to understand what the real issue is from this
> > description. I don't like the sound of processes leaking and if that is
> > happening, adding another exception to this list doesn't feel correct.
> > I was going to ask for a better explanation but looking at the code,
> > perhaps this error handling path just needs rewriting/improving with
> > more of the code in the finally, conditionally?
> > 
> > I just want to make sure we fix the real bug here.
> 
> Sorry for being unclear. I thought the backtrace would be too verbose.
> 
> The bug happens when runqemu startup fails:
> 
> poky/meta/lib/oeqa/targetcontrol.py:            raise RuntimeError("%s - FAILED to start qemu - check the task log and the boot log" % self.pn)
> 
> cooker processes do leak when the exceptions are not cought.
> Maybe these are not strictly related but it happens for me. It
> can be that cleanup happens but just slowly, and when I run
> other bitbake commands right after failure they connect to these
> leaked cooker processes which then behave badly, for example when
> build/tmp was already wiped.
> 

Sorry for the delay in looking at this patch. I'm a bit worried about
there being a leaked processes and wanted to understand if there was
other cleanup we should be doing.

Instead of this path, would it make sense to move the results.stop()
inside the finally? I'm worried that other forms of exception would
also leak processes.

Cheers,

Richard