Re: [Qemu-devel] [PATCH for-2.12 v2] iothread: workaround glib bug which hangs qmp-test

2018-04-09 Thread Stefan Hajnoczi
On Mon, Apr 09, 2018 at 04:39:56PM +0800, Peter Xu wrote:
> Free the AIO context earlier than the GMainContext (if we have) to
> workaround a glib2 bug that GSource context pointer is not cleared even
> if the context has already been destroyed (while it should).
> 
> The patch itself only changed the order to destroy the objects, no
> functional change at all. Without this workaround, we can encounter
> qmp-test hang with oob (and possibly any other use case when iothread is
> used with GMainContexts):
> 
>   #0  0x7f35ffe45334 in __lll_lock_wait () from /lib64/libpthread.so.0
>   #1  0x7f35ffe405d8 in _L_lock_854 () from /lib64/libpthread.so.0
>   #2  0x7f35ffe404a7 in pthread_mutex_lock () from /lib64/libpthread.so.0
>   #3  0x7f35fc5b9c9d in g_source_unref_internal (source=0x24f0600, 
> context=0x7f35f960, have_lock=0) at gmain.c:1685
>   #4  0x00aa6672 in aio_context_unref (ctx=0x24f0600) at 
> /root/qemu/util/async.c:497
>   #5  0x0065851c in iothread_instance_finalize (obj=0x24f0380) at 
> /root/qemu/iothread.c:129
>   #6  0x00962d79 in object_deinit (obj=0x24f0380, type=0x242e960) at 
> /root/qemu/qom/object.c:462
>   #7  0x00962e0d in object_finalize (data=0x24f0380) at 
> /root/qemu/qom/object.c:476
>   #8  0x00964146 in object_unref (obj=0x24f0380) at 
> /root/qemu/qom/object.c:924
>   #9  0x00965880 in object_finalize_child_property (obj=0x24ec640, 
> name=0x24efca0 "mon_iothread", opaque=0x24f0380) at 
> /root/qemu/qom/object.c:1436
>   #10 0x00962c33 in object_property_del_child (obj=0x24ec640, 
> child=0x24f0380, errp=0x0) at /root/qemu/qom/object.c:436
>   #11 0x00962d26 in object_unparent (obj=0x24f0380) at 
> /root/qemu/qom/object.c:455
>   #12 0x00658f00 in iothread_destroy (iothread=0x24f0380) at 
> /root/qemu/iothread.c:365
>   #13 0x004c67a8 in monitor_cleanup () at /root/qemu/monitor.c:4663
>   #14 0x00669e27 in main (argc=16, argv=0x7ffc8b1ae2f8, 
> envp=0x7ffc8b1ae380) at /root/qemu/vl.c:4749
> 
> The glib2 bug is fixed in commit 26056558b ("gmain: allow
> g_source_get_context() on destroyed sources", 2012-07-30), the first
> good version is glib2 2.33.10. So this error will be encountered before
> any glib version older than 2.33.10 (not including). Since we are still
> supporting even older glib versions, we may want this workaround.
> 
> Let's make sure we destroy the GSources first before its owner context
> until we drop support for glibs older than 2.33.10.
> 
> Signed-off-by: Peter Xu 
> ---
> v2:
> - verified the root cause of the bug, and enhance commit message and
>   comments correspondingly
> ---
>  iothread.c | 18 ++
>  1 file changed, 14 insertions(+), 4 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH for-2.12 v2] iothread: workaround glib bug which hangs qmp-test

2018-04-09 Thread Eric Blake
On 04/09/2018 03:39 AM, Peter Xu wrote:
> Free the AIO context earlier than the GMainContext (if we have) to
> workaround a glib2 bug that GSource context pointer is not cleared even
> if the context has already been destroyed (while it should).
> 
> The patch itself only changed the order to destroy the objects, no
> functional change at all. Without this workaround, we can encounter
> qmp-test hang with oob (and possibly any other use case when iothread is
> used with GMainContexts):
> 

> The glib2 bug is fixed in commit 26056558b ("gmain: allow
> g_source_get_context() on destroyed sources", 2012-07-30), the first
> good version is glib2 2.33.10. So this error will be encountered before
> any glib version older than 2.33.10 (not including). Since we are still
> supporting even older glib versions, we may want this workaround.

Grammar is awkward; I'd suggest:

...the first good version is glib2 2.33.10.  But we still support
building with glib as old as 2.28, so we need the workaround.

> 
> Let's make sure we destroy the GSources first before its owner context
> until we drop support for glibs older than 2.33.10.
> 
> Signed-off-by: Peter Xu 
> ---
> v2:
> - verified the root cause of the bug, and enhance commit message and
>   comments correspondingly

Reviewed-by: Eric Blake 

If Stefan is happy with your improved commit message, I'll queue this
for -rc3.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature