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.
---
libvirt-glib/libvirt-glib-event.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/libvirt-glib/libvirt-glib-event.c b/libvirt-glib/libvirt-glib-event.c
index 587a59e..657c1bf 100644
--- a/libvirt-glib/libvirt-glib-event.c
+++ b/libvirt-glib/libvirt-glib-event.c
@@ -87,7 +87,7 @@ struct gvir_event_handle
int watch;
int fd;
int events;
- int enabled;
+ int removed;
GIOChannel *channel;
guint source;
virEventHandleCallback cb;
@@ -99,6 +99,7 @@ struct gvir_event_timeout
{
int timer;
int interval;
+ int removed;
guint source;
virEventTimeoutCallback cb;
void *opaque;
@@ -195,7 +196,7 @@ gvir_event_handle_find(int watch)
continue;
}
- if (h->watch == watch) {
+ if ((h->watch == watch) && !h->removed) {
return h;
}
}
@@ -289,6 +290,11 @@ gvir_event_handle_remove(int watch)
g_source_remove(data->source);
data->source = 0;
data->events = 0;
+ /* since the actual watch deletion is done asynchronously, a handle_update 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(_event_handle_remove, data);
ret = 0;
@@ -358,7 +364,7 @@ gvir_event_timeout_find(int timer)
continue;
}
- if (t->timer == timer) {
+ if ((t->timer == timer) && !t->removed) {
return t;
}
}
@@ -441,6 +447,11 @@ gvir_event_timeout_remove(int timer)
g_source_remove(data->source);
data->source = 0;
+ /* since the actual timeout deletion is done asynchronously, a timeout_update 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(_event_timeout_remove, data);
ret = 0;
--
1.7.10.2