Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert() failure

[Markus Armbruster <armbru@redhat.com>]

Stefan Hajnoczi <stefanha@gmail.com> writes:

> On Tue, Feb 18, 2014 at 11:02:52AM +0100, Markus Armbruster wrote:
>> Markus Armbruster <armbru@redhat.com> writes:
>> 
>> > Stefan Hajnoczi <stefanha@redhat.com> writes:
>> >
>> >> The QEMU process stays running if the test case fails.  This patch fixes
>> >> the leak by installing a SIGABRT signal handler which invokes
>> >> qtest_end().
>> >>
>> >> In order to make that work for assertion failures during qtest_init(),
>> >> we need to initialize QTestState fields including file descriptors and
>> >> pids carefully.  qtest_quit() is then safe to call even during
>> >> qtest_init().
>> >>
>> >> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> >> ---
>> >>  tests/libqtest.c | 29 ++++++++++++++++++++++++++---
>> >>  1 file changed, 26 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/tests/libqtest.c b/tests/libqtest.c
>> >> index 8b2b2d7..09a0481 100644
>> >> --- a/tests/libqtest.c
>> >> +++ b/tests/libqtest.c
>> >> @@ -44,6 +44,7 @@ struct QTestState
>> >>      bool irq_level[MAX_IRQ];
>> >>      GString *rx;
>> >>      pid_t qemu_pid;  /* our child QEMU process */
>> >> +    struct sigaction sigact_old; /* restored on exit */
>> >>  };
>> >>  
>> >>  #define g_assert_no_errno(ret) do { \
>> >> @@ -88,6 +89,11 @@ static int socket_accept(int sock)
>> >>      return ret;
>> >>  }
>> >>  
>> >> +static void sigabrt_handler(int signo)
>> >> +{
>> >> +    qtest_end();
>> 
>> Don't you have to re-raise SIGABRT here, to actually terminate the
>> process?
>
> No.  POSIX says:
>
>   RETURN VALUE
>        The abort() function shall not return.
>
> (BTW the way to avoid that is using longjmp.)
>
> The Linux man page is more explicit:
>
>   If  the  SIGABRT  signal  is ignored, or caught by a handler that returns,
>   the abort() function will still terminate the process.  It does this by
>   restoring the default disposition for  SIGABRT and then raising the signal
>   for a second time.

Learn something new :)  Thanks!