[libvirt PATCH] Fix incorrect uses of g_clear_pointer() introduced in 8.1.0

[Mark Mielke]

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
glibc 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.
---
 src/qemu/qemu_agent.c   |  3 ++-
 src/qemu/qemu_monitor.c |  3 ++-
 src/util/vireventglib.c | 12 ++++++++----
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index f57a8d5f25..e6e92c7dc4 100644
--- 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 37bcbde31e..32c993a941 100644
--- 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 fc04d8f712..983787932f 100644
--- 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
-- 
2.36.1