Typo in the short commit message: against On Fri, Jul 17, 2015 at 04:01:21PM +0200, Fabiano Fidêncio wrote: > 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 > > _______________________________________________ > virt-tools-list mailing list > virt-tools-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/virt-tools-list _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
