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

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

On Tue, 2025-01-28 at 13:04 +0000, Richard Purdie via
lists.openembedded.org wrote:
> 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.

I've looked more at this and I don't understand how this patch works.

By adding the exception to the exception clause, it will trigger a
bb.error() call. The results.stop() call won't happen as results isn't
set in this failure.

The only significant difference would therefore be the results =
tc.results assignment. Does that really stop processes? If so, should
we always be doing that?

I'd like to understand what we need to do to "fix" things so we can
handle other exceptions.

Cheers,

Richard