[virt-tools-list] [virt-viewer][PATCH 4/6] events: protect "handles" and "timeouts" agins concurrent access

[Fabiano Fidêncio]

Timeout and Handle operations are done from an idle callback. However,
we cannot assume that all libvirt event calls (the callbacks passed to
virEventRegisterImpl) will be done from the mainloop thread. It's thus
possible that a libvirt event call will run a thread while one of the
callbacks runs.
Given that "handles" and "timeouts" arrays are shared among all threads,
we need to make sure we hold the "eventlock" mutex before modifying
them.

Based on
http://libvirt.org/git/?p=libvirt-glib.git;a=commit;h=924178f6b35735458b37d30303fe7bc753dde0b1

Related to: rhbz#1243228
---
 src/virt-viewer-events.c | 109 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 82 insertions(+), 27 deletions(-)

diff --git a/src/virt-viewer-events.c b/src/virt-viewer-events.c
index b92506c..ef2c317 100644
--- a/src/virt-viewer-events.c
+++ b/src/virt-viewer-events.c
@@ -33,6 +33,9 @@
 #include <libvirt/libvirt.h>
 
 #include "virt-viewer-events.h"
+#include "virt-glib-compat.h"
+
+GMutex *eventlock = NULL;
 
 struct virt_viewer_events_handle
 {
@@ -84,6 +87,9 @@ int virt_viewer_events_add_handle(int fd,
 {
     struct virt_viewer_events_handle *data;
     GIOCondition cond = 0;
+    int ret;
+
+    g_mutex_lock(eventlock);
 
     data = g_new0(struct virt_viewer_events_handle, 1);
 
@@ -114,7 +120,11 @@ int virt_viewer_events_add_handle(int fd,
 
     g_ptr_array_add(handles, data);
 
-    return data->watch;
+    ret = data->watch;
+
+    g_mutex_unlock(eventlock);
+
+    return ret;
 }
 
 static struct virt_viewer_events_handle *
@@ -144,19 +154,22 @@ static void
 virt_viewer_events_update_handle(int watch,
                                  int events)
 {
-    struct virt_viewer_events_handle *data = virt_viewer_events_find_handle(watch);
+    struct virt_viewer_events_handle *data;
+
+    g_mutex_lock(eventlock);
 
-    if (!data) {
+    data = virt_viewer_events_find_handle(watch);
+    if (data == NULL) {
         g_debug("Update for missing handle watch %d", watch);
-        return;
+        goto cleanup;
     }
 
-    if (events) {
+    if (events != 0) {
         GIOCondition cond = 0;
         if (events == data->events)
-            return;
+            goto cleanup;
 
-        if (data->source)
+        if (data->source != 0)
             g_source_remove(data->source);
 
         cond |= G_IO_HUP;
@@ -170,13 +183,16 @@ virt_viewer_events_update_handle(int watch,
                                       data);
         data->events = events;
     } else {
-        if (!data->source)
-            return;
+        if (data->source == 0)
+            goto cleanup;
 
         g_source_remove(data->source);
         data->source = 0;
         data->events = 0;
     }
+
+cleanup:
+    g_mutex_unlock(eventlock);
 }
 
 
@@ -191,7 +207,10 @@ virt_viewer_events_cleanup_handle(gpointer user_data)
     if (data->ff)
         (data->ff)(data->opaque);
 
+    g_mutex_lock(eventlock);
     g_ptr_array_remove_fast(handles, data);
+    g_mutex_unlock(eventlock);
+
     return FALSE;
 }
 
@@ -199,16 +218,20 @@ virt_viewer_events_cleanup_handle(gpointer user_data)
 static int
 virt_viewer_events_remove_handle(int watch)
 {
-    struct virt_viewer_events_handle *data = virt_viewer_events_find_handle(watch);
+    struct virt_viewer_events_handle *data;
+    int ret = -1;
 
-    if (!data) {
+    g_mutex_lock(eventlock);
+
+    data = virt_viewer_events_find_handle(watch);
+    if (data == NULL) {
         g_debug("Remove of missing watch %d", watch);
-        return -1;
+        goto cleanup;
     }
 
     g_debug("Remove handle %d %d", watch, data->fd);
 
-    if (data->source) {
+    if (data->source != 0) {
         g_source_remove(data->source);
         data->source = 0;
         data->events = 0;
@@ -220,7 +243,12 @@ virt_viewer_events_remove_handle(int watch)
      */
     data->removed = TRUE;
     g_idle_add(virt_viewer_events_cleanup_handle, data);
-    return 0;
+    ret = 0;
+
+cleanup:
+    g_mutex_unlock(eventlock);
+
+    return ret;
 }
 
 struct virt_viewer_events_timeout
@@ -255,6 +283,9 @@ virt_viewer_events_add_timeout(int interval,
                                virFreeCallback ff)
 {
     struct virt_viewer_events_timeout *data;
+    int ret;
+
+    g_mutex_lock(eventlock);
 
     data = g_new0(struct virt_viewer_events_timeout, 1);
 
@@ -272,7 +303,11 @@ virt_viewer_events_add_timeout(int interval,
 
     g_debug("Add timeout %p %d %p %p %d", data, interval, cb, opaque, data->timer);
 
-    return data->timer;
+    ret = data->timer;
+
+    g_mutex_unlock(eventlock);
+
+    return ret;
 }
 
 
@@ -304,30 +339,36 @@ static void
 virt_viewer_events_update_timeout(int timer,
                                   int interval)
 {
-    struct virt_viewer_events_timeout *data = virt_viewer_events_find_timeout(timer);
+    struct virt_viewer_events_timeout *data;
+
+    g_mutex_lock(eventlock);
 
-    if (!data) {
+    data = virt_viewer_events_find_timeout(timer);
+    if (data == NULL) {
         g_debug("Update of missing timer %d", timer);
-        return;
+        goto cleanup;
     }
 
     g_debug("Update timeout %p %d %d", data, timer, interval);
 
     if (interval >= 0) {
-        if (data->source)
-            return;
+        if (data->source != 0)
+            goto cleanup;
 
         data->interval = interval;
         data->source = g_timeout_add(data->interval,
                                      virt_viewer_events_dispatch_timeout,
                                      data);
     } else {
-        if (!data->source)
-            return;
+        if (data->source == 0)
+            goto cleanup;
 
         g_source_remove(data->source);
         data->source = 0;
     }
+
+cleanup:
+    g_mutex_unlock(eventlock);
 }
 
 
@@ -342,7 +383,10 @@ virt_viewer_events_cleanup_timeout(gpointer user_data)
     if (data->ff)
         (data->ff)(data->opaque);
 
+    g_mutex_lock(eventlock);
     g_ptr_array_remove_fast(timeouts, data);
+    g_mutex_unlock(eventlock);
+
     return FALSE;
 }
 
@@ -350,16 +394,21 @@ virt_viewer_events_cleanup_timeout(gpointer user_data)
 static int
 virt_viewer_events_remove_timeout(int timer)
 {
-    struct virt_viewer_events_timeout *data = virt_viewer_events_find_timeout(timer);
+    struct virt_viewer_events_timeout *data;
+    int ret = -1;
+
+    g_mutex_lock(eventlock);
 
-    if (!data) {
+    data = virt_viewer_events_find_timeout(timer);
+
+    if (data == NULL) {
         g_debug("Remove of missing timer %d", timer);
-        return -1;
+        goto cleanup;
     }
 
     g_debug("Remove timeout %p %d", data, timer);
 
-    if (data->source) {
+    if (data->source != 0) {
         g_source_remove(data->source);
         data->source = 0;
     }
@@ -370,11 +419,17 @@ virt_viewer_events_remove_timeout(int timer)
      */
     data->removed = TRUE;
     g_idle_add(virt_viewer_events_cleanup_timeout, data);
-    return 0;
+    ret = 0;
+
+cleanup:
+    g_mutex_unlock(eventlock);
+
+    return ret;
 }
 
 static gpointer event_register_once(gpointer data G_GNUC_UNUSED)
 {
+    eventlock = g_mutex_new();
     timeouts = g_ptr_array_new_with_free_func(g_free);
     handles = g_ptr_array_new_with_free_func(g_free);
     virEventRegisterImpl(virt_viewer_events_add_handle,
-- 
2.4.4