[libvirt PATCH 2/2] glib: Use safe glib event workaround in other event loops

[Daniel P. Berrangé]

On Thu, Mar 04, 2021 at 10:48:11AM +0100, Martin Kletzander wrote:
> Similarly to the crash workaround:
> 
>   commit 0db4743645b7a0611a3c0687f834205c9956f7fc
>   Author: Daniel P. Berrangé <berrange at redhat.com>
>   Date:   Tue Jul 28 16:52:47 2020 +0100
> 
>     util: avoid crash due to race in glib event loop code
> 
> we need to do this in the other event loop as crash in that one was also
> reported:
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=1931331
> 
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
>  src/qemu/qemu_agent.c     | 2 +-
>  src/qemu/qemu_monitor.c   | 2 +-
>  src/util/vireventthread.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index 9a74b802b89d..8b880d0d157c 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -533,7 +533,7 @@ qemuAgentUnregister(qemuAgentPtr agent)
>  {
>      if (agent->watch) {
>          g_source_destroy(agent->watch);
> -        g_source_unref(agent->watch);
> +        g_vir_source_unref_safe(agent->watch);
>          agent->watch = NULL;
>      }
>  }
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 73f337a6be53..b4f2641504f8 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -862,7 +862,7 @@ qemuMonitorUnregister(qemuMonitorPtr mon)
>  {
>      if (mon->watch) {
>          g_source_destroy(mon->watch);
> -        g_source_unref(mon->watch);
> +        g_vir_source_unref_safe(mon->watch);
>          mon->watch = NULL;
>      }
>  }

Upto here:

Reviewed-by: Daniel P. Berrangé <berrange at redhat.com>


> diff --git a/src/util/vireventthread.c b/src/util/vireventthread.c
> index 8342f420f666..e9d18d3acf2f 100644
> --- a/src/util/vireventthread.c
> +++ b/src/util/vireventthread.c
> @@ -123,7 +123,7 @@ virEventThreadWorker(void *opaque)
>  
>      g_main_loop_run(data->loop);
>  
> -    g_source_unref(running);
> +    g_vir_source_unref_safe(running);

This one isn't desirable as it'll cause a memory leak.  This method is the
very thread that was running the event loop, so there's no possibility
of a race condition. Also the loop has stopped running at this point,
so the idle source will never run. Just drop this hunk



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|