util: keep track of full GSource object not source ID number

The source ID number is an alternative way to identify a source that has
been added to a GMainContext. Internally when a source ID is given, glib
will lookup the corresponding GSource and use that. The use of a source
ID is racy in some cases though, because it is invalid to continue to
use an ID number after the GSource has been removed. It is thus safer
to use the GSource object directly and have full control over the ref
counting and thus cleanup.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 441f1502a6bedd353741bb9742710619428507f2..6031d9ab5eafe2c561495594248ef58305eb189f 100644 (file)
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -826,6 +826,7 @@ virNetClientIOEventTLS(int fd,
 static gboolean
 virNetClientTLSHandshake(virNetClientPtr client)
 {
+    g_autoptr(GSource) source = NULL;
     GIOCondition ev;
     int ret;
 
@@ -840,10 +841,10 @@ virNetClientTLSHandshake(virNetClientPtr client)
     else
         ev = G_IO_OUT;
 
-    virEventGLibAddSocketWatch(virNetSocketGetFD(client->sock),
-                               ev,
-                               client->eventCtx,
-                               virNetClientIOEventTLS, client, NULL);
+    source = virEventGLibAddSocketWatch(virNetSocketGetFD(client->sock),
+                                        ev,
+                                        client->eventCtx,
+                                        virNetClientIOEventTLS, client, NULL);
 
     return TRUE;
 }
@@ -882,6 +883,7 @@ int virNetClientSetTLSSession(virNetClientPtr client,
     int ret;
     char buf[1];
     int len;
+    g_autoptr(GSource) source = NULL;
 
 #ifndef WIN32
     sigset_t oldmask, blockedsigs;
@@ -934,10 +936,10 @@ int virNetClientSetTLSSession(virNetClientPtr client,
      * etc.  If we make the grade, it will send us a '\1' byte.
      */
 
-    virEventGLibAddSocketWatch(virNetSocketGetFD(client->sock),
-                               G_IO_IN,
-                               client->eventCtx,
-                               virNetClientIOEventTLSConfirm, client, NULL);
+    source = virEventGLibAddSocketWatch(virNetSocketGetFD(client->sock),
+                                        G_IO_IN,
+                                        client->eventCtx,
+                                        virNetClientIOEventTLSConfirm, client, NULL);
 
 #ifndef WIN32
     /* Block SIGWINCH from interrupting poll in curses programs */
@@ -1617,6 +1619,7 @@ static int virNetClientIOEventLoop(virNetClientPtr client,
 #endif /* !WIN32 */
         int timeout = -1;
         virNetMessagePtr msg = NULL;
+        g_autoptr(GSource) source = NULL;
         GIOCondition ev = 0;
         struct virNetClientIOEventData data = {
             .client = client,
@@ -1651,10 +1654,10 @@ static int virNetClientIOEventLoop(virNetClientPtr client,
         if (client->nstreams)
             ev |= G_IO_IN;
 
-        virEventGLibAddSocketWatch(virNetSocketGetFD(client->sock),
-                                   ev,
-                                   client->eventCtx,
-                                   virNetClientIOEventFD, &data, NULL);
+        source = virEventGLibAddSocketWatch(virNetSocketGetFD(client->sock),
+                                            ev,
+                                            client->eventCtx,
+                                            virNetClientIOEventFD, &data, NULL);
 
         /* Release lock while poll'ing so other threads
          * can stuff themselves on the queue */

diff --git a/src/util/vireventglib.c b/src/util/vireventglib.c
index 803332a6f8639d86ae27f1c932b5d0adf3d3535d..6e334b33982f03a17ba6be60e4e2c574a16597fe 100644 (file)
--- a/src/util/vireventglib.c
+++ b/src/util/vireventglib.c
@@ -45,7 +45,7 @@ struct virEventGLibHandle
     int fd;
     int events;
     int removed;
-    guint source;
+    GSource *source;
     virEventHandleCallback cb;
     void *opaque;
     virFreeCallback ff;
@@ -56,7 +56,7 @@ struct virEventGLibTimeout
     int timer;
     int interval;
     int removed;
-    guint source;
+    GSource *source;
     virEventTimeoutCallback cb;
     void *opaque;
     virFreeCallback ff;
@@ -210,23 +210,25 @@ virEventGLibHandleUpdate(int watch,
         if (events == data->events)
             goto cleanup;
 
-        if (data->source != 0) {
-            VIR_DEBUG("Removed old handle watch=%d", data->source);
-            g_source_remove(data->source);
+        if (data->source != NULL) {
+            VIR_DEBUG("Removed old handle source=%p", data->source);
+            g_source_destroy(data->source);
+            g_source_unref(data->source);
         }
 
         data->source = virEventGLibAddSocketWatch(
             data->fd, cond, NULL, virEventGLibHandleDispatch, data, NULL);
 
         data->events = events;
-        VIR_DEBUG("Added new handle watch=%d", data->source);
+        VIR_DEBUG("Added new handle source=%p", data->source);
     } else {
-        if (data->source == 0)
+        if (data->source == NULL)
             goto cleanup;
 
-        VIR_DEBUG("Removed old handle watch=%d", data->source);
-        g_source_remove(data->source);
-        data->source = 0;
+        VIR_DEBUG("Removed old handle source=%p", data->source);
+        g_source_destroy(data->source);
+        g_source_unref(data->source);
+        data->source = NULL;
         data->events = 0;
     }
 
@@ -272,9 +274,10 @@ virEventGLibHandleRemove(int watch)
     VIR_DEBUG("Remove handle data=%p watch=%d fd=%d",
               data, watch, data->fd);
 
-    if (data->source != 0) {
-        g_source_remove(data->source);
-        data->source = 0;
+    if (data->source != NULL) {
+        g_source_destroy(data->source);
+        g_source_unref(data->source);
+        data->source = NULL;
         data->events = 0;
     }
 
@@ -309,6 +312,22 @@ virEventGLibTimeoutDispatch(void *opaque)
     return TRUE;
 }
 
+
+static GSource *
+virEventGLibTimeoutCreate(int interval,
+                          struct virEventGLibTimeout *data)
+{
+    GSource *source = g_timeout_source_new(interval);
+
+    g_source_set_callback(source,
+                          virEventGLibTimeoutDispatch,
+                          data, NULL);
+    g_source_attach(source, NULL);
+
+    return source;
+}
+
+
 static int
 virEventGLibTimeoutAdd(int interval,
                        virEventTimeoutCallback cb,
@@ -327,9 +346,7 @@ virEventGLibTimeoutAdd(int interval,
     data->opaque = opaque;
     data->ff = ff;
     if (interval >= 0)
-        data->source = g_timeout_add(interval,
-                                     virEventGLibTimeoutDispatch,
-                                     data);
+        data->source = virEventGLibTimeoutCreate(interval, data);
 
     g_ptr_array_add(timeouts, data);
 
@@ -390,19 +407,20 @@ virEventGLibTimeoutUpdate(int timer,
     VIR_DEBUG("Update timeout data=%p timer=%d interval=%d ms", data, timer, interval);
 
     if (interval >= 0) {
-        if (data->source != 0)
-            g_source_remove(data->source);
+        if (data->source != NULL) {
+            g_source_destroy(data->source);
+            g_source_unref(data->source);
+        }
 
         data->interval = interval;
-        data->source = g_timeout_add(data->interval,
-                                     virEventGLibTimeoutDispatch,
-                                     data);
+        data->source = virEventGLibTimeoutCreate(interval, data);
     } else {
-        if (data->source == 0)
+        if (data->source == NULL)
             goto cleanup;
 
-        g_source_remove(data->source);
-        data->source = 0;
+        g_source_destroy(data->source);
+        g_source_unref(data->source);
+        data->source = NULL;
     }
 
  cleanup:
@@ -448,9 +466,10 @@ virEventGLibTimeoutRemove(int timer)
     VIR_DEBUG("Remove timeout data=%p timer=%d",
               data, timer);
 
-    if (data->source != 0) {
-        g_source_remove(data->source);
-        data->source = 0;
+    if (data->source != NULL) {
+        g_source_destroy(data->source);
+        g_source_unref(data->source);
+        data->source = NULL;
     }
 
     /* since the actual timeout deletion is done asynchronously, a timeoutUpdate call may

diff --git a/src/util/vireventglibwatch.c b/src/util/vireventglibwatch.c
index 178707f6b704914cf154291cf03714294a1e8ebf..b7f3a8786a83c21caa46b2c1b6931c3c07f0a644 100644 (file)
--- a/src/util/vireventglibwatch.c
+++ b/src/util/vireventglibwatch.c
@@ -233,17 +233,20 @@ GSource *virEventGLibCreateSocketWatch(int fd,
 #endif /* WIN32 */
 
 
-guint virEventGLibAddSocketWatch(int fd,
-                                 GIOCondition condition,
-                                 GMainContext *context,
-                                 virEventGLibSocketFunc func,
-                                 gpointer opaque,
-                                 GDestroyNotify notify)
+GSource *
+virEventGLibAddSocketWatch(int fd,
+                           GIOCondition condition,
+                           GMainContext *context,
+                           virEventGLibSocketFunc func,
+                           gpointer opaque,
+                           GDestroyNotify notify)
 {
-    g_autoptr(GSource) source = NULL;
+    GSource *source = NULL;
 
     source = virEventGLibCreateSocketWatch(fd, condition);
     g_source_set_callback(source, (GSourceFunc)func, opaque, notify);
 
-    return g_source_attach(source, context);
+    g_source_attach(source, context);
+
+    return source;
 }

diff --git a/src/util/vireventglibwatch.h b/src/util/vireventglibwatch.h
index 2f7e61cfbaac75132c0be62abbd1d277ee5808cc..f57be1f503de6d85007c85f7dc5fe4ebf1a5adcc 100644 (file)
--- a/src/util/vireventglibwatch.h
+++ b/src/util/vireventglibwatch.h
@@ -40,9 +40,10 @@ typedef gboolean (*virEventGLibSocketFunc)(int fd,
                                            GIOCondition condition,
                                            gpointer data);
 
-guint virEventGLibAddSocketWatch(int fd,
-                                 GIOCondition condition,
-                                 GMainContext *context,
-                                 virEventGLibSocketFunc func,
-                                 gpointer opaque,
-                                 GDestroyNotify notify);
+GSource *virEventGLibAddSocketWatch(int fd,
+                                    GIOCondition condition,
+                                    GMainContext *context,
+                                    virEventGLibSocketFunc func,
+                                    gpointer opaque,
+                                    GDestroyNotify notify)
+    G_GNUC_WARN_UNUSED_RESULT;