src: ensure GSource background unref happens in correct event loop

The g_idle_add function adds a callback to the primary GMainContext.

To workaround the GSource unref bugs, we need to add our callbacks
to the GMainContext that is associated with the GSource being
unref'd. Thus code using the per-VM virEventThread must use its
private GMainContext.

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/meson.build b/meson.build
index 69a7b4c88e7e2bfd14c4fee7a96af8aaa8d407ec..ea93a2a8ecb93979dbda225bc6b59005a4eade98 100644 (file)
--- a/meson.build
+++ b/meson.build
@@ -1049,9 +1049,6 @@ if host_machine.system() == 'windows'
 else
   gio_dep = dependency('gio-unix-2.0', version: '>=' + glib_version)
 endif
-# GLib event loop race workaround in glibcompat.h, remove when minimum required
-# glib is >= 2.64.0
-glib_crash_workaround = glib_dep.version().version_compare('<2.64.0')
 glib_dep = declare_dependency(
   dependencies: [ glib_dep, gobject_dep, gio_dep ],
 )

diff --git a/src/libvirt_glib_crash_workaround.syms b/src/libvirt_glib_crash_workaround.syms
deleted file mode 100644 (file)
index 249058b..0000000
--- a/src/libvirt_glib_crash_workaround.syms
+++ /dev/null
@@ -1,11 +0,0 @@
-#
-# Private symbols specific for pre-2.64.0 GLib workaround
-#
-
-# util/glibcompat.h
-virEventGLibSourceUnrefIdle;
-
-# Let emacs know we want case-insensitive sorting
-# Local Variables:
-# sort-fold-case: t
-# End:

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 767d665613bdced6a0dacf63fef0590457f649fd..526dcee11a4945b6133697cc2d918550dd905406 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1733,6 +1733,7 @@ virStorageSourceUpdatePhysicalSize;
 # util/glibcompat.h
 vir_g_canonicalize_filename;
 vir_g_fsync;
+vir_g_source_unref;
 vir_g_strdup_printf;
 vir_g_strdup_vprintf;
 

diff --git a/src/meson.build b/src/meson.build
index 70a5a83eea3eda471a071cb121e33a59c21d0873..c7ff9e978cd1c948f625cfba73900fa13f0292ec 100644 (file)
--- a/src/meson.build
+++ b/src/meson.build
@@ -124,13 +124,6 @@ else
   sym_files += 'libvirt_libssh2.syms'
 endif
 
-if glib_crash_workaround
-  used_sym_files += 'libvirt_glib_crash_workaround.syms'
-else
-  sym_files += 'libvirt_glib_crash_workaround.syms'
-endif
-
-
 # variables filled by subdirectories
 
 libvirt_libs = []

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 01ba2c78c1c201e2858a8b9e83b652ecec30446c..787a7bb41c4e3e2c8353cc724e55f3e68688441d 100644 (file)
--- 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_vir_source_unref_safe(agent->watch);
+        vir_g_source_unref(agent->watch, agent->context);
         agent->watch = NULL;
     }
 }

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index b4f2641504f8276ae8c354aa950f433c55e18059..f6cd9d9edadcc579e718b7bc405e3608da4737f5 100644 (file)
--- 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_vir_source_unref_safe(mon->watch);
+        vir_g_source_unref(mon->watch, mon->context);
         mon->watch = NULL;
     }
 }

diff --git a/src/util/glibcompat.c b/src/util/glibcompat.c
index fe19ffa87b3fe789e5c22db7e6c87fc04ba69347..eb6dcc0111b9bb968a6e45a659cc45f31d297df0 100644 (file)
--- a/src/util/glibcompat.c
+++ b/src/util/glibcompat.c
@@ -231,9 +231,14 @@ vir_g_strdup_vprintf(const char *msg, va_list args)
  *
  * Drop when min glib >= 2.64.0
  */
-#if GLIB_CHECK_VERSION(2, 64, 0) != TRUE
+#if GLIB_CHECK_VERSION(2, 64, 0)
+void vir_g_source_unref(GSource *src, GMainContext *ctx G_GNUC_UNUSED)
+{
+    g_source_unref(src);
+}
+#else
 
-gboolean
+static gboolean
 virEventGLibSourceUnrefIdle(gpointer data)
 {
     GSource *src = data;
@@ -243,4 +248,15 @@ virEventGLibSourceUnrefIdle(gpointer data)
     return FALSE;
 }
 
+void vir_g_source_unref(GSource *src, GMainContext *ctx)
+{
+    GSource *idle = g_idle_source_new();
+
+    g_source_set_callback(idle, virEventGLibSourceUnrefIdle, src, NULL);
+
+    g_source_attach(idle, ctx);
+
+    g_source_unref(idle);
+}
+
 #endif

diff --git a/src/util/glibcompat.h b/src/util/glibcompat.h
index 9c528432742ba184d2e45a9b175649b2041508df..697687b967f8416e6d7b3a4ae57520576b97851b 100644 (file)
--- a/src/util/glibcompat.h
+++ b/src/util/glibcompat.h
@@ -85,13 +85,4 @@ char *vir_g_strdup_vprintf(const char *msg, va_list args)
 #undef g_fsync
 #define g_fsync vir_g_fsync
 
-/* Drop when min glib >= 2.64.0 */
-#if GLIB_CHECK_VERSION(2, 64, 0)
-# define g_vir_source_unref_safe(source) g_source_unref(source)
-#else
-# define g_vir_source_unref_safe(source) g_idle_add(virEventGLibSourceUnrefIdle, source)
-
-gboolean
-virEventGLibSourceUnrefIdle(gpointer data);
-
-#endif
+void vir_g_source_unref(GSource *src, GMainContext *ctx);

diff --git a/src/util/vireventglib.c b/src/util/vireventglib.c
index 88e3ec6d5dbb84225fc0423e498384d451fe5bff..f3e5a344b058029391c1391f2b867de6398cd6cb 100644 (file)
--- a/src/util/vireventglib.c
+++ b/src/util/vireventglib.c
@@ -214,7 +214,7 @@ virEventGLibHandleUpdate(int watch,
         if (data->source != NULL) {
             VIR_DEBUG("Removed old handle source=%p", data->source);
             g_source_destroy(data->source);
-            g_vir_source_unref_safe(data->source);
+            vir_g_source_unref(data->source, NULL);
         }
 
         data->source = virEventGLibAddSocketWatch(
@@ -228,7 +228,7 @@ virEventGLibHandleUpdate(int watch,
 
         VIR_DEBUG("Removed old handle source=%p", data->source);
         g_source_destroy(data->source);
-        g_vir_source_unref_safe(data->source);
+        vir_g_source_unref(data->source, NULL);
         data->source = NULL;
         data->events = 0;
     }
@@ -277,7 +277,7 @@ virEventGLibHandleRemove(int watch)
 
     if (data->source != NULL) {
         g_source_destroy(data->source);
-        g_vir_source_unref_safe(data->source);
+        vir_g_source_unref(data->source, NULL);
         data->source = NULL;
         data->events = 0;
     }
@@ -410,7 +410,7 @@ virEventGLibTimeoutUpdate(int timer,
     if (interval >= 0) {
         if (data->source != NULL) {
             g_source_destroy(data->source);
-            g_vir_source_unref_safe(data->source);
+            vir_g_source_unref(data->source, NULL);
         }
 
         data->interval = interval;
@@ -420,7 +420,7 @@ virEventGLibTimeoutUpdate(int timer,
             goto cleanup;
 
         g_source_destroy(data->source);
-        g_vir_source_unref_safe(data->source);
+        vir_g_source_unref(data->source, NULL);
         data->source = NULL;
     }
 
@@ -469,7 +469,7 @@ virEventGLibTimeoutRemove(int timer)
 
     if (data->source != NULL) {
         g_source_destroy(data->source);
-        g_vir_source_unref_safe(data->source);
+        vir_g_source_unref(data->source, NULL);
         data->source = NULL;
     }