diff mbox series

[v2,1/2] tests/qtest: fix registration of ABRT handler for QEMU cleanup

Message ID 20220513154906.206715-2-berrange@redhat.com
State New
Headers show
Series tests/libqtest: fix cleanup of QEMU processes and add robustness | expand

Commit Message

Daniel P. Berrangé May 13, 2022, 3:49 p.m. UTC
qtest_init registers a hook to cleanup the running QEMU process
should g_assert() fire before qtest_quit is called. When the first
hook is registered, it is supposed to triggere registration of the
SIGABRT handler. Unfortunately the logic in hook_list_is_empty is
inverted, so the SIGABRT handler never gets registered, unless
2 or more QEMU processes are run concurrently. This caused qtest
to leak QEMU processes anytime g_assert triggers.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qtest/libqtest.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Thomas Huth May 13, 2022, 4:08 p.m. UTC | #1
On 13/05/2022 17.49, Daniel P. Berrangé wrote:
> qtest_init registers a hook to cleanup the running QEMU process
> should g_assert() fire before qtest_quit is called. When the first
> hook is registered, it is supposed to triggere registration of the
> SIGABRT handler. Unfortunately the logic in hook_list_is_empty is
> inverted, so the SIGABRT handler never gets registered, unless
> 2 or more QEMU processes are run concurrently. This caused qtest
> to leak QEMU processes anytime g_assert triggers.

Ouch, thanks for spotting it!

> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/qtest/libqtest.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 228357f1ea..4a4697c0d1 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -197,11 +197,11 @@ static bool hook_list_is_empty(GHookList *hook_list)
>       GHook *hook = g_hook_first_valid(hook_list, TRUE);
>   
>       if (!hook) {
> -        return false;
> +        return true;
>       }
>   
>       g_hook_unref(hook_list, hook);
> -    return true;
> +    return false;
>   }

Reviewed-by: Thomas Huth <thuth@redhat.com>
diff mbox series

Patch

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 228357f1ea..4a4697c0d1 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -197,11 +197,11 @@  static bool hook_list_is_empty(GHookList *hook_list)
     GHook *hook = g_hook_first_valid(hook_list, TRUE);
 
     if (!hook) {
-        return false;
+        return true;
     }
 
     g_hook_unref(hook_list, hook);
-    return true;
+    return false;
 }
 
 void qtest_add_abrt_handler(GHookFunc fn, const void *data)