Fix incorrect uses of g_clear_pointer() introduced in 8.1.0

This is a partial revert of 87a43a907f0ad4897a28ad7c216bc70f37270b93

The change to use g_clear_pointer() in more places was accidentally
applied to cases involving vir_g_source_unref().

In some cases, the ordering of g_source_destroy() and
vir_g_source_unref() was reversed, which resulted in the source being
marked as destroyed, after it is already unreferenced. This
use-after-free case might work in many cases, but with versions of
glib older than 2.64.0 it may defer unref to run within the main
thread to avoid a race condition, which creates a large distance
between the g_source_unref() and g_source_destroy().

In some cases, the call to vir_g_source_unref() was replaced with a
second call to g_source_destroy(), leading to a memory leak or worse.

In our experience, the symptoms were that use of libvirt-python became
slower over time, with OpenStack nova-compute initially taking around
one second to periodically query the host PCI devices, and within an
hour it was taking over a minute to complete the same operation, until
it is was eventually running this query back-to-back, resulting in the
nova-compute process consuming 100% of one CPU thread, losing its
RabbitMQ connection frequently, and showing up as down to the control
plane.

Signed-off-by: Mark Mielke <mark.mielke@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index f57a8d5f255831e93147d69a38a33d7d9c3a3afd..e6e92c7dc4ec92d973e2f2b0383b167e5958ca4e 100644 (file)
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -452,8 +452,9 @@ static void
 qemuAgentUnregister(qemuAgent *agent)
 {
     if (agent->watch) {
+        g_source_destroy(agent->watch);
         vir_g_source_unref(agent->watch, agent->context);
-        g_clear_pointer(&agent->watch, g_source_destroy);
+        agent->watch = NULL;
     }
 }
 

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 37bcbde31e0e79fc638795f6bb2c9a742abb9da9..32c993a941f0e8ea2bf7b8541d9f4621d2b7a134 100644 (file)
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -794,8 +794,9 @@ void
 qemuMonitorUnregister(qemuMonitor *mon)
 {
     if (mon->watch) {
+        g_source_destroy(mon->watch);
         vir_g_source_unref(mon->watch, mon->context);
-        g_clear_pointer(&mon->watch, g_source_destroy);
+        mon->watch = NULL;
     }
 }
 

diff --git a/src/util/vireventglib.c b/src/util/vireventglib.c
index fc04d8f7120461a313a098e00d571816a45b2830..983787932fab625fed10510849aee55521bf0293 100644 (file)
--- a/src/util/vireventglib.c
+++ b/src/util/vireventglib.c
@@ -228,7 +228,8 @@ virEventGLibHandleUpdate(int watch,
 
         VIR_DEBUG("Removed old handle source=%p", data->source);
         g_source_destroy(data->source);
-        g_clear_pointer(&data->source, g_source_destroy);
+        vir_g_source_unref(data->source, NULL);
+        data->source = NULL;
         data->events = 0;
     }
 
@@ -275,8 +276,9 @@ virEventGLibHandleRemove(int watch)
               data, watch, data->fd);
 
     if (data->source != NULL) {
+        g_source_destroy(data->source);
         vir_g_source_unref(data->source, NULL);
-        g_clear_pointer(&data->source, g_source_destroy);
+        data->source = NULL;
         data->events = 0;
     }
 
@@ -417,8 +419,9 @@ virEventGLibTimeoutUpdate(int timer,
         if (data->source == NULL)
             goto cleanup;
 
+        g_source_destroy(data->source);
         vir_g_source_unref(data->source, NULL);
-        g_clear_pointer(&data->source, g_source_destroy);
+        data->source = NULL;
     }
 
  cleanup:
@@ -465,8 +468,9 @@ virEventGLibTimeoutRemove(int timer)
               data, timer);
 
     if (data->source != NULL) {
+        g_source_destroy(data->source);
         vir_g_source_unref(data->source, NULL);
-        g_clear_pointer(&data->source, g_source_destroy);
+        data->source = NULL;
     }
 
     /* since the actual timeout deletion is done asynchronously, a timeoutUpdate call may