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

[Peter Maydell]

On Mon, 6 Jan 2020 at 15:22, Marc-André Lureau
<address@hidden> wrote:
>
> Hi
>
> On Mon, Jan 6, 2020 at 7:03 PM Peter Maydell <address@hidden> wrote:
> >
> > 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).

> >  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.
>
> in/if, I suppose

No; it means "before anything in this new thread specifically" as
opposed to "before anything in the whole process". (This comment is
verbatim copied from the main iothread.c, incidentally).

Thanks for the review.

-- PMM