events: don't reschedule deleted timeouts/watches

The deletion of libvirt timeouts/watches is done in 2 steps:
- the first step is synchronous and unregisters the timeout/watch
  from glib mainloop
- the second step is asynchronous and triggered from the first step.
  It releases the memory used for bookkeeping for the timeout/watch
  being deleted

This is done this way to avoid some possible deadlocks when
reentering the sync callback while freeing the memory associated
with the timeout/watch.

However, it's possible to call gvir_event_update_handle after
gvir_event_remove_handle but before _event_handle_remove does
the final cleanup. When this happen, _update_handle will reregister
the handle with glib mainloop, and bad things will happen when
a glib callback is triggered for this event after _event_handle_remove
has freed the memory associated with this handle watch.

This commit marks the timeouts and watches as removed in the
synchronous _remove callback and makes sure removed timeouts/watches
are ignored in _update callbacks.

Based on commit 3e73e0cee977fb20dd29db3ccfe85b00cc386c43 from
libvirt-glib.
Original author: Christophe Fergeau <cfergeau@redhat.com>

Related to: rhbz#1243228

1 files changed, 14 insertions, 3 deletions

diff --git a/src/virt-viewer-events.c b/src/virt-viewer-events.c
index 65db605..1655a70 100644
--- a/src/virt-viewer-events.c
+++ b/src/virt-viewer-events.c
@@ -42,7 +42,7 @@ struct virt_viewer_events_handle
     int watch;
     int fd;
     int events;
-    int enabled;
+    int removed;
     GIOChannel *channel;
     guint source;
     virEventHandleCallback cb;
@@ -140,7 +140,7 @@ virt_viewer_events_find_handle(int watch)
             continue;
         }
 
-        if (h->watch == watch) {
+        if ((h->watch == watch) && !h->removed) {
             return h;
         }
     }
@@ -240,6 +240,11 @@ virt_viewer_events_remove_handle(int watch)
     data->source = 0;
     data->events = 0;
 
+    /* since the actual watch deletion is done asynchronously, a update_handle call may
+     * reschedule the watch before it's fully deleted, that's why we need to mark it as
+     * 'removed' to prevent reuse
+     */
+    data->removed = TRUE;
     g_idle_add(virt_viewer_events_cleanup_handle, data);
     ret = 0;
 
@@ -252,6 +257,7 @@ struct virt_viewer_events_timeout
 {
     int timer;
     int interval;
+    int removed;
     guint source;
     virEventTimeoutCallback cb;
     void *opaque;
@@ -322,7 +328,7 @@ virt_viewer_events_find_timeout(int timer)
             continue;
         }
 
-        if (t->timer == timer) {
+        if ((t->timer == timer) && !t->removed) {
             return t;
         }
     }
@@ -411,6 +417,11 @@ virt_viewer_events_remove_timeout(int timer)
     g_source_remove(data->source);
     data->source = 0;
 
+    /* since the actual timeout deletion is done asynchronously, a update_timeout call may
+     * reschedule the timeout before it's fully deleted, that's why we need to mark it as
+     * 'removed' to prevent reuse
+     */
+    data->removed = TRUE;
     g_idle_add(virt_viewer_events_cleanup_timeout, data);
     ret = 0;