On Tue, Sep 26, 2017 at 01:44:39PM +0800, Peter Xu wrote:
> Hi,
>
> Generally speaking this is a question about glib, but I would like to
> see how the list sees this before throwing this question to glib
> community in case I made severe mistake.
>
> I encountered one glib warning when start to use internal iothread:
>
> (qemu-system-x86_64:19925): GLib-CRITICAL **: g_source_remove_poll: assertion
> '!SOURCE_DESTROYED (source)' failed
>
> This will be triggered as long as we create one private iothread and
> enables gcontext of it (by calling iothread_get_g_main_context() at
> least once on the private iothread).
>
> The warning is generated when quitting QEMU in following path (please
> ignore unknown function names, they only appear in a private tree, but
> the logic is the same):
>
> #0 0x00007ff0c2bb4180 in g_source_remove_poll () at /lib64/libglib-2.0.so.0
> #1 0x00005634b0fe8bc9 in aio_set_fd_handler (ctx=0x5634b365ca90, fd=22,
> is_external=false, io_read=0x0, io_write=0x0, io_poll=0x0,
> opaque=0x5634b365cb3c) at /root/git/qemu/util/aio-posix.c:226
> #2 0x00005634b0fe8ebc in aio_set_event_notifier (ctx=0x5634b365ca90,
> notifier=0x5634b365cb3c, is_external=false, io_read=0x0, io_poll=0x0) at
> /root/git/qemu/util/aio-posix.c:299
> #3 0x00005634b0fe5174 in aio_ctx_finalize (source=0x5634b365ca90) at
> /root/git/qemu/util/async.c:296
> #4 0x00007ff0c2bb39a6 in g_source_unref_internal () at
> /lib64/libglib-2.0.so.0
> #5 0x00005634b0fe5757 in aio_context_unref (ctx=0x5634b365ca90) at
> /root/git/qemu/util/async.c:484
> #6 0x00005634b0c39642 in iothread_instance_finalize (obj=0x5634b36cfa40) at
> /root/git/qemu/iothread.c:127
> #7 0x00005634b0ede36e in object_deinit (obj=0x5634b36cfa40,
> type=0x5634b3543c60) at /root/git/qemu/qom/object.c:453
> #8 0x00005634b0ede3e0 in object_finalize (data=0x5634b36cfa40) at
> /root/git/qemu/qom/object.c:467
> #9 0x00005634b0edf361 in object_unref (obj=0x5634b36cfa40) at
> /root/git/qemu/qom/object.c:902
> #10 0x00005634b0ee0607 in object_finalize_child_property (obj=0x5634b369d3b0,
> name=0x5634b36eb900 "monitor_io_thr", opaque=0x5634b36cfa40) at
> /root/git/qemu/qom/object.c:1407
> #11 0x00005634b0ede25d in object_property_del_child (obj=0x5634b369d3b0,
> child=0x5634b36cfa40, errp=0x0) at /root/git/qemu/qom/object.c:427
> #12 0x00005634b0ede33d in object_unparent (obj=0x5634b36cfa40) at
> /root/git/qemu/qom/object.c:446
> #13 0x00005634b0c39f44 in iothread_destroy (iothread=0x5634b36cfa40) at
> /root/git/qemu/iothread.c:383
> #14 0x00005634b0ae920f in monitor_io_thread_destroy () at
> /root/git/qemu/monitor.c:4416
> #15 0x00005634b0ae9302 in monitor_cleanup () at /root/git/qemu/monitor.c:4439
> #16 0x00005634b0c496cb in main (argc=17, argv=0x7ffcc5b88918,
> envp=0x7ffcc5b889a8) at /root/git/qemu/vl.c:4889
>
> It's on the path during destruction of AioContext, where we called
> g_source_remove_poll() in the destructor. When reach here, AioContext
> has been detached from the gcontext already, so the assertion is
> triggered.
>
> I'll copy some code for easier reference from glib:
>
> void
> g_source_remove_poll (GSource *source,
> GPollFD *fd)
> {
> GMainContext *context;
>
> g_return_if_fail (source != NULL);
> g_return_if_fail (fd != NULL);
> g_return_if_fail (!SOURCE_DESTROYED (source)); <---- this assertion fails
>
> context = source->context;
>
> if (context)
> LOCK_CONTEXT (context);
>
> source->poll_fds = g_slist_remove (source->poll_fds, fd);
>
> if (context)
> {
> if (!SOURCE_BLOCKED (source))
> g_main_context_remove_poll_unlocked (context, fd);
> UNLOCK_CONTEXT (context);
> }
> }
>
> I'm translating this assertion failure into: "when removing one
> GSource from the polled GSource array, the GSource should still be
> attached to its GMainContext". But does this really matter? Why
> can't we remove one GSource from the poll array even if the source is
> detached already? After all if we see following code in
> g_source_remove_poll() we have already taken special care when
> source->context == NULL happens.
>
> Any thoughts? TIA.
It seems glib chose this cleanup order:
GPollFD < GSource < GMainLoop < GMainContext
Even if glib could be modified to allow what you want, QEMU needs to
work with old glib versions.
I suggest modifying iothread.c to implement glib's required cleanup
order in the finalize function:
aio_context_unref(); <-- the GSource
g_main_loop_unref();
g_main_context_unref();
Stefan
