From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:44037)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1WFm2L-0002TE-Hd
	for qemu-devel@nongnu.org; Tue, 18 Feb 2014 09:52:47 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1WFm2F-0004yt-Fu
	for qemu-devel@nongnu.org; Tue, 18 Feb 2014 09:52:41 -0500
Received: from mx1.redhat.com ([209.132.183.28]:43452)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1WFm2F-0004yb-54
	for qemu-devel@nongnu.org; Tue, 18 Feb 2014 09:52:35 -0500
From: Markus Armbruster <armbru@redhat.com>
References: <1392651898-16749-1-git-send-email-stefanha@redhat.com>
	<1392651898-16749-4-git-send-email-stefanha@redhat.com>
	<878ut9u1k4.fsf@blackfin.pond.sub.org>
	<87sirgivqr.fsf@blackfin.pond.sub.org>
	<20140218143839.GA15348@stefanha-thinkpad.redhat.com>
Date: Tue, 18 Feb 2014 15:52:21 +0100
In-Reply-To: <20140218143839.GA15348@stefanha-thinkpad.redhat.com> (Stefan
	Hajnoczi's message of "Tue, 18 Feb 2014 15:38:39 +0100")
Message-ID: <87ob2479sq.fsf@blackfin.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH 3/3] qtest: kill QEMU process on g_assert()
	failure
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>, Anthony Liguori <aliguori@amazon.com>, Andreas Faerber <afaerber@suse.de>, Stefan Hajnoczi <stefanha@redhat.com>, qemu-devel@nongnu.org

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!