qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] tests/iothread: Always connect iothread GSource to a GMainContex


From: Peter Maydell
Subject: [PATCH] tests/iothread: Always connect iothread GSource to a GMainContext
Date: Mon, 6 Jan 2020 14:45:52 +0000

On older versions of glib (anything prior to glib commit 0f056ebe
from May 2019), the implementation of g_source_ref() and
g_source_unref() is not threadsafe for a GSource which is not
attached to a GMainContext.

QEMU's real iothread.c implementation always attaches its
iothread->ctx's GSource to a GMainContext created for that iothread,
so it is OK, but the simple test framework implementation in
tests/iothread.c was not doing this.  This was causing intermittent
assertion failures in the test-aio-multithread subtest
"/aio/multi/mutex/contended" test on the BSD hosts.  (It's unclear
why only BSD seems to have been affected -- perhaps a combination of
the specific glib version being used in the VMs and their happening
to run on a host with a lot of CPUs).

Borrow the iothread_init_gcontext() from the real iothread.c
and add the corresponding cleanup code and the calls to
g_main_context_push/pop_thread_default() so we actually use
the GMainContext we create.

Signed-off-by: Peter Maydell <address@hidden>
---
I don't really have a good understanding of the glib APIs here,
so I'm mostly just cribbing code from the real iothread.c;
review by people who do know the glib/iothread stuff better
welcomed. It does seem to fix the intermittent test failure
on NetBSD, at least, where we were running into an assertion
failure because a g_source_unref() incorrectly thought it
had decremented the refcount to 0 and should delete a context
that was actually still in use.

 tests/iothread.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tests/iothread.c b/tests/iothread.c
index 13c9fdcd8df..d3a2ee9a014 100644
--- a/tests/iothread.c
+++ b/tests/iothread.c
@@ -21,6 +21,8 @@
 
 struct IOThread {
     AioContext *ctx;
+    GMainContext *worker_context;
+    GMainLoop *main_loop;
 
     QemuThread thread;
     QemuMutex init_done_lock;
@@ -35,6 +37,17 @@ AioContext *qemu_get_current_aio_context(void)
     return my_iothread ? my_iothread->ctx : qemu_get_aio_context();
 }
 
+static void iothread_init_gcontext(IOThread *iothread)
+{
+    GSource *source;
+
+    iothread->worker_context = g_main_context_new();
+    source = aio_get_g_source(iothread_get_aio_context(iothread));
+    g_source_attach(source, iothread->worker_context);
+    g_source_unref(source);
+    iothread->main_loop = g_main_loop_new(iothread->worker_context, TRUE);
+}
+
 static void *iothread_run(void *opaque)
 {
     IOThread *iothread = opaque;
@@ -44,6 +57,20 @@ static void *iothread_run(void *opaque)
     my_iothread = iothread;
     qemu_mutex_lock(&iothread->init_done_lock);
     iothread->ctx = aio_context_new(&error_abort);
+
+    /*
+     * We must connect the ctx to a GMainContext, because in older versions
+     * of glib the g_source_ref()/unref() functions are not threadsafe
+     * on sources without a context.
+     */
+    iothread_init_gcontext(iothread);
+
+    /*
+     * g_main_context_push_thread_default() must be called before anything
+     * in this new thread uses glib.
+     */
+    g_main_context_push_thread_default(iothread->worker_context);
+
     qemu_cond_signal(&iothread->init_done_cond);
     qemu_mutex_unlock(&iothread->init_done_lock);
 
@@ -51,6 +78,7 @@ static void *iothread_run(void *opaque)
         aio_poll(iothread->ctx, true);
     }
 
+    g_main_context_pop_thread_default(iothread->worker_context);
     rcu_unregister_thread();
     return NULL;
 }
@@ -66,6 +94,8 @@ void iothread_join(IOThread *iothread)
 {
     aio_bh_schedule_oneshot(iothread->ctx, iothread_stop_bh, iothread);
     qemu_thread_join(&iothread->thread);
+    g_main_context_unref(iothread->worker_context);
+    g_main_loop_unref(iothread->main_loop);
     qemu_cond_destroy(&iothread->init_done_cond);
     qemu_mutex_destroy(&iothread->init_done_lock);
     aio_context_unref(iothread->ctx);
-- 
2.20.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]