[virt-tools-list] [virt-viewer][PATCH] events: "backport" a few patches from libvirt-glib-events to virt-viewer -- missing patch
This patch should part of the previous series that I sent [0]. [0]: https://www.redhat.com/archives/virt-tools-list/2015-July/msg00063.html Fabiano Fidêncio (1): events: Don't leak GIOChannel when destroying IO handle src/virt-viewer-events.c | 4 1 file changed, 4 insertions(+) -- 2.4.4 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer][PATCH 4/6] events: protect "handles" and "timeouts" agins concurrent access
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
>
> #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);
> +
Re: [virt-tools-list] [virt-viewer][PATCH 0/6] events: "backport" a few patches from libvirt-glib-events to virt-viewer
An alternative to this series is to revert https://git.fedorahosted.org/cgit/virt-viewer.git/commit/?id=296f91c (which would mean a new dependency not available in EL6). libvirt-glib mainloop implementation has matured a lot in the mean time as shown by this series ;) Christophe On Fri, Jul 17, 2015 at 04:01:17PM +0200, Fabiano Fidêncio wrote: > In order to fix rhbz#1243228, I am "backporting" a few fixes from > libvirt-glib/libvirt-glib-events.c to virt-viewer, as the events > file used for both seems to be pretty much the same (with the same > issues as well, sometimes fixed in one, sometimes fixed in the other). > > All patches have a link for the relevant commit in the libvirt-glib. > > These patches were already tested by the reporter of rhbz#1243228. > > Fabiano Fidêncio (6): > events: Register event using GOnce to avoid multiple initializations > events: remove timeout and handle from arrays > glib-compat: Add g_mutex_new() > events: protect "handles" and "timeouts" agins concurrent access > events: Don't create glib IO watch for disabled handles > events: Allow zero timeouts for timer > > src/virt-glib-compat.h | 4 + > src/virt-viewer-events.c | 224 > +-- > 2 files changed, 163 insertions(+), 65 deletions(-) > > -- > 2.4.4 > > ___ > virt-tools-list mailing list > virt-tools-list@redhat.com > https://www.redhat.com/mailman/listinfo/virt-tools-list pgpvRA1yIXGnA.pgp Description: PGP signature ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer][PATCH 6/6] events: Allow zero timeouts for timer
ACK, though this one comes much sooner in libvirt-glib history.
Christophe
On Fri, Jul 17, 2015 at 04:01:23PM +0200, Fabiano Fidêncio wrote:
> In libvirt, it's perfectly possible and widely used to have disabled
> timers (timeout=-1) and fire them up 'randomly' with timeout=0.
> However, with current mapping into glib mainloop it's not possible
> and causing troubles.
>
> Based on
> http://libvirt.org/git/?p=libvirt-glib.git;a=commit;h=a40a1732e0d53fcc44b8d348cec97152dafd2b88
>
> Related to: rhbz#1243228
> ---
> src/virt-viewer-events.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/virt-viewer-events.c b/src/virt-viewer-events.c
> index c6442f6..d36dd3a 100644
> --- a/src/virt-viewer-events.c
> +++ b/src/virt-viewer-events.c
> @@ -355,7 +355,7 @@ virt_viewer_events_update_timeout(int timer,
>
> if (interval >= 0) {
> if (data->source != 0)
> -goto cleanup;
> +g_source_remove(data->source);
>
> data->interval = interval;
> data->source = g_timeout_add(data->interval,
> --
> 2.4.4
>
> ___
> virt-tools-list mailing list
> virt-tools-list@redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
pgpNTTRZYlxHL.pgp
Description: PGP signature
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer][PATCH 2/6] events: remove timeout and handle from arrays
On Fri, Jul 17, 2015 at 6:15 PM, Christophe Fergeau wrote:
> Hey,
>
> On Fri, Jul 17, 2015 at 04:01:19PM +0200, Fabiano Fidêncio wrote:
>> Based on
>> http://libvirt.org/git/?p=libvirt-glib.git;a=commit;h=cff5f1c46f4b9661e112b85159fb58ae473a9a89
>
> I'd tend to c&p the initial log too
Not sure if the initial log would make sense here. But I can c&p it, no problem.
>
>>
>> Related to: rhbz#1243228
>> ---
>> src/virt-viewer-events.c | 97
>> +++-
>> 1 file changed, 63 insertions(+), 34 deletions(-)
>>
>> diff --git a/src/virt-viewer-events.c b/src/virt-viewer-events.c
>> index 6154353..b92506c 100644
>> --- a/src/virt-viewer-events.c
>> +++ b/src/virt-viewer-events.c
>> @@ -39,7 +39,7 @@ struct virt_viewer_events_handle
>> int watch;
>> int fd;
>> int events;
>> -int enabled;
>> +int removed;
>
> This (and other parts of this commit) are
> https://libvirt.org/git/?p=libvirt-glib.git;a=commitdiff;h=3e73e0cee977fb20dd29db3ccfe85b00cc386c43
> they should go in a separate commit
As it was not exactly cherry-picked from the libvirt-glib commits, I
don't see a good reason for not having all the fixes squashed when
they make sense to be squashed.
>
>> GIOChannel *channel;
>> guint source;
>> virEventHandleCallback cb;
>> @@ -48,8 +48,7 @@ struct virt_viewer_events_handle
>> };
>>
>> static int nextwatch = 1;
>> -static unsigned int nhandles = 0;
>> -static struct virt_viewer_events_handle **handles = NULL;
>> +GPtrArray *handles;
>
> static GPtrArray
>
>>
>> static gboolean
>> virt_viewer_events_dispatch_handle(GIOChannel *source G_GNUC_UNUSED,
>> @@ -86,9 +85,7 @@ int virt_viewer_events_add_handle(int fd,
>> struct virt_viewer_events_handle *data;
>> GIOCondition cond = 0;
>>
>> -handles = g_realloc(handles, sizeof(*handles)*(nhandles+1));
>> -data = g_malloc(sizeof(*data));
>> -memset(data, 0, sizeof(*data));
>> +data = g_new0(struct virt_viewer_events_handle, 1);
>>
>> if (events & VIR_EVENT_HANDLE_READABLE)
>> cond |= G_IO_IN;
>> @@ -115,7 +112,7 @@ int virt_viewer_events_add_handle(int fd,
>>virt_viewer_events_dispatch_handle,
>>data);
>>
>> -handles[nhandles++] = data;
>> +g_ptr_array_add(handles, data);
>>
>> return data->watch;
>> }
>> @@ -123,10 +120,22 @@ int virt_viewer_events_add_handle(int fd,
>> static struct virt_viewer_events_handle *
>> virt_viewer_events_find_handle(int watch)
>
> any reason for not keeping the guint *idx arg added in the source
> commit?
Yeah, as you mentioned, I squashed
https://libvirt.org/git/?p=libvirt-glib.git;a=commitdiff;h=1fb34633ef3b318ea678b775d5e47debc98d2184
>
>> {
>> -unsigned int i;
>> -for (i = 0 ; i < nhandles ; i++)
>> -if (handles[i]->watch == watch)
>> -return handles[i];
>> +guint i;
>> +
>> +g_return_val_if_fail(handles != NULL, NULL);
>> +
>> +for (i = 0; i < handles->len; i++) {
>> +struct virt_viewer_events_handle *h = g_ptr_array_index(handles, i);
>> +
>> +if (h == NULL) {
>> +g_warn_if_reached();
>> +continue;
>> +}
>> +
>> +if ((h->watch == watch) && !h->removed) {
>> +return h;
>> +}
>> +}
>>
>> return NULL;
>> }
>> @@ -182,7 +191,7 @@ virt_viewer_events_cleanup_handle(gpointer user_data)
>> if (data->ff)
>> (data->ff)(data->opaque);
>>
>> -free(data);
>> +g_ptr_array_remove_fast(handles, data);
>> return FALSE;
>> }
>>
>> @@ -199,13 +208,17 @@ virt_viewer_events_remove_handle(int watch)
>>
>> g_debug("Remove handle %d %d", watch, data->fd);
>>
>> -if (!data->source)
>> -return -1;
>> -
>> -g_source_remove(data->source);
>> -data->source = 0;
>> -data->events = 0;
>> +if (data->source) {
>> +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 befure it's fully deleted, that's why we need
>> to mar it as
>
> 'before' 'mark'
>
>
>> + * 'removed' to prevent reuse
>> + */
>> +data->removed = TRUE;
>> g_idle_add(virt_viewer_events_cleanup_handle, data);
>> return 0;
>> }
>> @@ -214,6 +227,7 @@ struct virt_viewer_events_timeout
>> {
>> int timer;
>> int interval;
>> +int removed;
>> guint source;
>> virEventTimeoutCallback cb;
>> void *opaque;
>> @@ -222,8 +236,7 @@ struct virt_viewer_events_timeout
>>
>>
>> static int nexttimer = 1;
>> -static unsigned int ntimeouts = 0;
>> -static struct virt_viewer_events_timeout **timeouts = NULL;
>> +GPtrArray *timeouts;
>
> static
>
>>
>> static gboolean
>> virt_viewer_events_dispatch_timeout(void *opaque)
>> @@ -243,9 +256,7 @@ virt_viewer_events_add_timeout(int interval,
Re: [virt-tools-list] [virt-viewer][PATCH 5/6] events: Don't create glib IO watch for disabled handles
ACK.
On Fri, Jul 17, 2015 at 04:01:22PM +0200, Fabiano Fidêncio wrote:
> It's possible to create a handle to watch for file events which do not
> watch for any file event. Such a handle can be enabled later with
> gvir_event_handle_update() by setting some conditions to watch for.
>
> When a handle is disabled after it has been created,
> gvir_event_handle_update() makes sure it removes the corresponding
> gvir_event_handle::source IO watch if any was set.
> gvir_event_handle_add() will always create a gvir_event_handle::source
> IO watch even if the handle is not watching for any events.
>
> This commit makes consistent by only creating a watch with g_io_add_watch()
> when the caller asked to watch for some events.
>
> Based on
> http://libvirt.org/git/?p=libvirt-glib.git;a=commit;h=d71c143936a35cd6c3f23ae0cbf7f3215d944051
>
> Related to: rhbz#1243228
> ---
> src/virt-viewer-events.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/src/virt-viewer-events.c b/src/virt-viewer-events.c
> index ef2c317..c6442f6 100644
> --- a/src/virt-viewer-events.c
> +++ b/src/virt-viewer-events.c
> @@ -113,10 +113,12 @@ int virt_viewer_events_add_handle(int fd,
>
> g_debug("Add handle %d %d %p", data->fd, events, data->opaque);
>
> -data->source = g_io_add_watch(data->channel,
> - cond,
> - virt_viewer_events_dispatch_handle,
> - data);
> +if (events != 0) {
> +data->source = g_io_add_watch(data->channel,
> + cond,
> + virt_viewer_events_dispatch_handle,
> + data);
> +}
>
> g_ptr_array_add(handles, data);
>
> --
> 2.4.4
>
> ___
> virt-tools-list mailing list
> virt-tools-list@redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
pgpxPOKJkqD1t.pgp
Description: PGP signature
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer][PATCH] events: Don't leak GIOChannel when destroying IO handle
ACK. On Fri, Jul 17, 2015 at 06:33:51PM +0200, Fabiano Fidêncio wrote: > virt_viewer_events_add_handle() creates a GIOChannel in order to watch > the fd it was given for changes. > virt_viewer_events_remove_handle() is freeing all the resources > allocated by virt_viewer_events_add_handle() expect for this GIOChannel. > This commit adds the needed g_io_channel_unref() call to > virt_viewer_events_remove_handle(). > > Based on > http://libvirt.org/git/?p=libvirt-glib.git;a=commit;h=8e95b8d25a3eee6316aff2f83b0c449aaf10984a > > Related to: rhbz#1243228 > --- > src/virt-viewer-events.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/src/virt-viewer-events.c b/src/virt-viewer-events.c > index d36dd3a..6cdc56a 100644 > --- a/src/virt-viewer-events.c > +++ b/src/virt-viewer-events.c > @@ -239,6 +239,10 @@ virt_viewer_events_remove_handle(int watch) > data->events = 0; > } > > +g_warn_if_fail(data->channel != NULL); > +g_io_channel_unref(data->channel); > +data->channel = NULL; > + > /* since the actual watch deletion is done asynchronously, a > handle_update call may > * reschedule the watch befure it's fully deleted, that's why we need to > mar it as > * 'removed' to prevent reuse > -- > 2.4.4 > > ___ > virt-tools-list mailing list > virt-tools-list@redhat.com > https://www.redhat.com/mailman/listinfo/virt-tools-list pgpKOmJzsVTX_.pgp Description: PGP signature ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer][PATCH 4/6] events: protect "handles" and "timeouts" agins concurrent access
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
This really is
https://libvirt.org/git/?p=libvirt-glib.git;a=commitdiff;h=f1fe67da2dac6a249f796535b8dbd155d5741ad7
with
http://libvirt.org/git/?p=libvirt-glib.git;a=commit;h=924178f6b35735458b37d30303fe7bc753dde0b1
and
https://libvirt.org/git/?p=libvirt-glib.git;a=commitdiff;h=dd17c3cc587c73a8c915238f9d9a3e200e89c93f
squashed in.
>
> 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
>
> #include "virt-viewer-events.h"
> +#include "virt-glib-compat.h"
> +
> +GMutex *eventlock = NULL;
static GMutex *eventlock
>
> 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) {
Please don't change if (!data) to your preferred style, this causes
unneeded diff output, and unneeded divergence from the libvirt-glib code
> g_debug("Update for missing handle watch %d", watch);
> -return;
> +goto cleanup;
> }
>
> -if (events) {
> +if (events != 0) {
ditto
> GIOCondition cond = 0;
> if (events == data->events)
> -return;
> +goto cleanup;
>
> -if (data->source)
> +if (data->source != 0)
ditto
> 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) {
Here I believe you squashed in
https://libvirt.org/git/?p=libvirt-glib.git;a=commitdiff;h=79699d73e6e1b7218e3bd8349d176752f86128b9
> 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);
> +
> +re
[virt-tools-list] [virt-viewer][PATCH] events: Don't leak GIOChannel when destroying IO handle
virt_viewer_events_add_handle() creates a GIOChannel in order to watch the fd it was given for changes. virt_viewer_events_remove_handle() is freeing all the resources allocated by virt_viewer_events_add_handle() expect for this GIOChannel. This commit adds the needed g_io_channel_unref() call to virt_viewer_events_remove_handle(). Based on http://libvirt.org/git/?p=libvirt-glib.git;a=commit;h=8e95b8d25a3eee6316aff2f83b0c449aaf10984a Related to: rhbz#1243228 --- src/virt-viewer-events.c | 4 1 file changed, 4 insertions(+) diff --git a/src/virt-viewer-events.c b/src/virt-viewer-events.c index d36dd3a..6cdc56a 100644 --- a/src/virt-viewer-events.c +++ b/src/virt-viewer-events.c @@ -239,6 +239,10 @@ virt_viewer_events_remove_handle(int watch) data->events = 0; } +g_warn_if_fail(data->channel != NULL); +g_io_channel_unref(data->channel); +data->channel = NULL; + /* since the actual watch deletion is done asynchronously, a handle_update call may * reschedule the watch befure it's fully deleted, that's why we need to mar it as * 'removed' to prevent reuse -- 2.4.4 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer][PATCH 2/6] events: remove timeout and handle from arrays
On Fri, Jul 17, 2015 at 06:15:19PM +0200, Christophe Fergeau wrote: > > @@ -115,7 +112,7 @@ int virt_viewer_events_add_handle(int fd, > >virt_viewer_events_dispatch_handle, > >data); > > > > -handles[nhandles++] = data; > > +g_ptr_array_add(handles, data); > > > > return data->watch; > > } > > @@ -123,10 +120,22 @@ int virt_viewer_events_add_handle(int fd, > > static struct virt_viewer_events_handle * > > virt_viewer_events_find_handle(int watch) > > any reason for not keeping the guint *idx arg added in the source > commit? > Ah ok you squashed https://libvirt.org/git/?p=libvirt-glib.git;a=commitdiff;h=1fb34633ef3b318ea678b775d5e47debc98d2184 in Christophe pgpIW0sxKMHav.pgp Description: PGP signature ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer][PATCH 3/6] glib-compat: Add g_mutex_new()
On Fri, Jul 17, 2015 at 04:01:20PM +0200, Fabiano Fidêncio wrote: > Related to: rhbz#1243228 > --- > src/virt-glib-compat.h | 4 > 1 file changed, 4 insertions(+) > > diff --git a/src/virt-glib-compat.h b/src/virt-glib-compat.h > index 2c887e9..17c33f0 100644 > --- a/src/virt-glib-compat.h > +++ b/src/virt-glib-compat.h > @@ -76,6 +76,10 @@ GByteArray *g_byte_array_new_take (guint8 *data, gsize > len); > #define G_SOURCE_REMOVE FALSE > #endif > > +#if GLIB_CHECK_VERSION(2, 31, 0) > +#define g_mutex_new() g_new0(GMutex, 1) > +#endif > + ACK > G_END_DECLS > > #endif // _VIRT_GLIB_COMPAT_H > -- > 2.4.4 > > ___ > virt-tools-list mailing list > virt-tools-list@redhat.com > https://www.redhat.com/mailman/listinfo/virt-tools-list pgpGG6e0XJBC7.pgp Description: PGP signature ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer][PATCH 1/6] events: Register event using GOnce to avoid multiple initializations
ACK
On Fri, Jul 17, 2015 at 04:01:18PM +0200, Fabiano Fidêncio wrote:
> Based on
> http://libvirt.org/git/?p=libvirt-glib.git;a=commit;h=8f8d9ce5238dbcbce40aa04ba55b8c55f97c79c0
>
> Related to: rhbz#1243228
> ---
> src/virt-viewer-events.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/src/virt-viewer-events.c b/src/virt-viewer-events.c
> index 3b5a136..6154353 100644
> --- a/src/virt-viewer-events.c
> +++ b/src/virt-viewer-events.c
> @@ -346,14 +346,22 @@ virt_viewer_events_remove_timeout(int timer)
> return 0;
> }
>
> -
> -void virt_viewer_events_register(void) {
> +static gpointer event_register_once(gpointer data G_GNUC_UNUSED)
> +{
> virEventRegisterImpl(virt_viewer_events_add_handle,
> virt_viewer_events_update_handle,
> virt_viewer_events_remove_handle,
> virt_viewer_events_add_timeout,
> virt_viewer_events_update_timeout,
> virt_viewer_events_remove_timeout);
> +
> +return NULL;
> +}
> +
> +void virt_viewer_events_register(void) {
> +static GOnce once = G_ONCE_INIT;
> +
> +g_once(&once, event_register_once, NULL);
> }
>
> /*
> --
> 2.4.4
>
> ___
> virt-tools-list mailing list
> virt-tools-list@redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
pgpEi3EYn_IyE.pgp
Description: PGP signature
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer][PATCH 2/6] events: remove timeout and handle from arrays
Hey,
On Fri, Jul 17, 2015 at 04:01:19PM +0200, Fabiano Fidêncio wrote:
> Based on
> http://libvirt.org/git/?p=libvirt-glib.git;a=commit;h=cff5f1c46f4b9661e112b85159fb58ae473a9a89
I'd tend to c&p the initial log too
>
> Related to: rhbz#1243228
> ---
> src/virt-viewer-events.c | 97
> +++-
> 1 file changed, 63 insertions(+), 34 deletions(-)
>
> diff --git a/src/virt-viewer-events.c b/src/virt-viewer-events.c
> index 6154353..b92506c 100644
> --- a/src/virt-viewer-events.c
> +++ b/src/virt-viewer-events.c
> @@ -39,7 +39,7 @@ struct virt_viewer_events_handle
> int watch;
> int fd;
> int events;
> -int enabled;
> +int removed;
This (and other parts of this commit) are
https://libvirt.org/git/?p=libvirt-glib.git;a=commitdiff;h=3e73e0cee977fb20dd29db3ccfe85b00cc386c43
they should go in a separate commit
> GIOChannel *channel;
> guint source;
> virEventHandleCallback cb;
> @@ -48,8 +48,7 @@ struct virt_viewer_events_handle
> };
>
> static int nextwatch = 1;
> -static unsigned int nhandles = 0;
> -static struct virt_viewer_events_handle **handles = NULL;
> +GPtrArray *handles;
static GPtrArray
>
> static gboolean
> virt_viewer_events_dispatch_handle(GIOChannel *source G_GNUC_UNUSED,
> @@ -86,9 +85,7 @@ int virt_viewer_events_add_handle(int fd,
> struct virt_viewer_events_handle *data;
> GIOCondition cond = 0;
>
> -handles = g_realloc(handles, sizeof(*handles)*(nhandles+1));
> -data = g_malloc(sizeof(*data));
> -memset(data, 0, sizeof(*data));
> +data = g_new0(struct virt_viewer_events_handle, 1);
>
> if (events & VIR_EVENT_HANDLE_READABLE)
> cond |= G_IO_IN;
> @@ -115,7 +112,7 @@ int virt_viewer_events_add_handle(int fd,
>virt_viewer_events_dispatch_handle,
>data);
>
> -handles[nhandles++] = data;
> +g_ptr_array_add(handles, data);
>
> return data->watch;
> }
> @@ -123,10 +120,22 @@ int virt_viewer_events_add_handle(int fd,
> static struct virt_viewer_events_handle *
> virt_viewer_events_find_handle(int watch)
any reason for not keeping the guint *idx arg added in the source
commit?
> {
> -unsigned int i;
> -for (i = 0 ; i < nhandles ; i++)
> -if (handles[i]->watch == watch)
> -return handles[i];
> +guint i;
> +
> +g_return_val_if_fail(handles != NULL, NULL);
> +
> +for (i = 0; i < handles->len; i++) {
> +struct virt_viewer_events_handle *h = g_ptr_array_index(handles, i);
> +
> +if (h == NULL) {
> +g_warn_if_reached();
> +continue;
> +}
> +
> +if ((h->watch == watch) && !h->removed) {
> +return h;
> +}
> +}
>
> return NULL;
> }
> @@ -182,7 +191,7 @@ virt_viewer_events_cleanup_handle(gpointer user_data)
> if (data->ff)
> (data->ff)(data->opaque);
>
> -free(data);
> +g_ptr_array_remove_fast(handles, data);
> return FALSE;
> }
>
> @@ -199,13 +208,17 @@ virt_viewer_events_remove_handle(int watch)
>
> g_debug("Remove handle %d %d", watch, data->fd);
>
> -if (!data->source)
> -return -1;
> -
> -g_source_remove(data->source);
> -data->source = 0;
> -data->events = 0;
> +if (data->source) {
> +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 befure it's fully deleted, that's why we need to
> mar it as
'before' 'mark'
> + * 'removed' to prevent reuse
> + */
> +data->removed = TRUE;
> g_idle_add(virt_viewer_events_cleanup_handle, data);
> return 0;
> }
> @@ -214,6 +227,7 @@ struct virt_viewer_events_timeout
> {
> int timer;
> int interval;
> +int removed;
> guint source;
> virEventTimeoutCallback cb;
> void *opaque;
> @@ -222,8 +236,7 @@ struct virt_viewer_events_timeout
>
>
> static int nexttimer = 1;
> -static unsigned int ntimeouts = 0;
> -static struct virt_viewer_events_timeout **timeouts = NULL;
> +GPtrArray *timeouts;
static
>
> static gboolean
> virt_viewer_events_dispatch_timeout(void *opaque)
> @@ -243,9 +256,7 @@ virt_viewer_events_add_timeout(int interval,
> {
> struct virt_viewer_events_timeout *data;
>
> -timeouts = g_realloc(timeouts, sizeof(*timeouts)*(ntimeouts+1));
> -data = g_malloc(sizeof(*data));
> -memset(data, 0, sizeof(*data));
> +data = g_new0(struct virt_viewer_events_timeout, 1);
>
> data->timer = nexttimer++;
> data->interval = interval;
> @@ -257,7 +268,7 @@ virt_viewer_events_add_timeout(int interval,
> virt_viewer_events_dispatch_timeout,
> data);
>
> -timeouts[ntimeouts++] = data;
> +g_ptr_arra
Re: [virt-tools-list] [virt-viewer][PATCH 0/6] events: "backport" a few patches from libvirt-glib-events to virt-viewer
On Fri, Jul 17, 2015 at 04:01:17PM +0200, Fabiano Fidêncio wrote: > In order to fix rhbz#1243228, I am "backporting" a few fixes from > libvirt-glib/libvirt-glib-events.c to virt-viewer, as the events > file used for both seems to be pretty much the same (with the same > issues as well, sometimes fixed in one, sometimes fixed in the other). Do you mean virt-viewer has some fixes which are not in libvirt-glib? Christophe pgpci7_E4yPQb.pgp Description: PGP signature ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-viewer][PATCH 0/6] events: "backport" a few patches from libvirt-glib-events to virt-viewer
In order to fix rhbz#1243228, I am "backporting" a few fixes from libvirt-glib/libvirt-glib-events.c to virt-viewer, as the events file used for both seems to be pretty much the same (with the same issues as well, sometimes fixed in one, sometimes fixed in the other). All patches have a link for the relevant commit in the libvirt-glib. These patches were already tested by the reporter of rhbz#1243228. Fabiano Fidêncio (6): events: Register event using GOnce to avoid multiple initializations events: remove timeout and handle from arrays glib-compat: Add g_mutex_new() events: protect "handles" and "timeouts" agins concurrent access events: Don't create glib IO watch for disabled handles events: Allow zero timeouts for timer src/virt-glib-compat.h | 4 + src/virt-viewer-events.c | 224 +-- 2 files changed, 163 insertions(+), 65 deletions(-) -- 2.4.4 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-viewer][PATCH 5/6] events: Don't create glib IO watch for disabled handles
It's possible to create a handle to watch for file events which do not
watch for any file event. Such a handle can be enabled later with
gvir_event_handle_update() by setting some conditions to watch for.
When a handle is disabled after it has been created,
gvir_event_handle_update() makes sure it removes the corresponding
gvir_event_handle::source IO watch if any was set.
gvir_event_handle_add() will always create a gvir_event_handle::source
IO watch even if the handle is not watching for any events.
This commit makes consistent by only creating a watch with g_io_add_watch()
when the caller asked to watch for some events.
Based on
http://libvirt.org/git/?p=libvirt-glib.git;a=commit;h=d71c143936a35cd6c3f23ae0cbf7f3215d944051
Related to: rhbz#1243228
---
src/virt-viewer-events.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/virt-viewer-events.c b/src/virt-viewer-events.c
index ef2c317..c6442f6 100644
--- a/src/virt-viewer-events.c
+++ b/src/virt-viewer-events.c
@@ -113,10 +113,12 @@ int virt_viewer_events_add_handle(int fd,
g_debug("Add handle %d %d %p", data->fd, events, data->opaque);
-data->source = g_io_add_watch(data->channel,
- cond,
- virt_viewer_events_dispatch_handle,
- data);
+if (events != 0) {
+data->source = g_io_add_watch(data->channel,
+ cond,
+ virt_viewer_events_dispatch_handle,
+ data);
+}
g_ptr_array_add(handles, data);
--
2.4.4
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-viewer][PATCH 6/6] events: Allow zero timeouts for timer
In libvirt, it's perfectly possible and widely used to have disabled
timers (timeout=-1) and fire them up 'randomly' with timeout=0.
However, with current mapping into glib mainloop it's not possible
and causing troubles.
Based on
http://libvirt.org/git/?p=libvirt-glib.git;a=commit;h=a40a1732e0d53fcc44b8d348cec97152dafd2b88
Related to: rhbz#1243228
---
src/virt-viewer-events.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/virt-viewer-events.c b/src/virt-viewer-events.c
index c6442f6..d36dd3a 100644
--- a/src/virt-viewer-events.c
+++ b/src/virt-viewer-events.c
@@ -355,7 +355,7 @@ virt_viewer_events_update_timeout(int timer,
if (interval >= 0) {
if (data->source != 0)
-goto cleanup;
+g_source_remove(data->source);
data->interval = interval;
data->source = g_timeout_add(data->interval,
--
2.4.4
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-viewer][PATCH 2/6] events: remove timeout and handle from arrays
Based on
http://libvirt.org/git/?p=libvirt-glib.git;a=commit;h=cff5f1c46f4b9661e112b85159fb58ae473a9a89
Related to: rhbz#1243228
---
src/virt-viewer-events.c | 97 +++-
1 file changed, 63 insertions(+), 34 deletions(-)
diff --git a/src/virt-viewer-events.c b/src/virt-viewer-events.c
index 6154353..b92506c 100644
--- a/src/virt-viewer-events.c
+++ b/src/virt-viewer-events.c
@@ -39,7 +39,7 @@ struct virt_viewer_events_handle
int watch;
int fd;
int events;
-int enabled;
+int removed;
GIOChannel *channel;
guint source;
virEventHandleCallback cb;
@@ -48,8 +48,7 @@ struct virt_viewer_events_handle
};
static int nextwatch = 1;
-static unsigned int nhandles = 0;
-static struct virt_viewer_events_handle **handles = NULL;
+GPtrArray *handles;
static gboolean
virt_viewer_events_dispatch_handle(GIOChannel *source G_GNUC_UNUSED,
@@ -86,9 +85,7 @@ int virt_viewer_events_add_handle(int fd,
struct virt_viewer_events_handle *data;
GIOCondition cond = 0;
-handles = g_realloc(handles, sizeof(*handles)*(nhandles+1));
-data = g_malloc(sizeof(*data));
-memset(data, 0, sizeof(*data));
+data = g_new0(struct virt_viewer_events_handle, 1);
if (events & VIR_EVENT_HANDLE_READABLE)
cond |= G_IO_IN;
@@ -115,7 +112,7 @@ int virt_viewer_events_add_handle(int fd,
virt_viewer_events_dispatch_handle,
data);
-handles[nhandles++] = data;
+g_ptr_array_add(handles, data);
return data->watch;
}
@@ -123,10 +120,22 @@ int virt_viewer_events_add_handle(int fd,
static struct virt_viewer_events_handle *
virt_viewer_events_find_handle(int watch)
{
-unsigned int i;
-for (i = 0 ; i < nhandles ; i++)
-if (handles[i]->watch == watch)
-return handles[i];
+guint i;
+
+g_return_val_if_fail(handles != NULL, NULL);
+
+for (i = 0; i < handles->len; i++) {
+struct virt_viewer_events_handle *h = g_ptr_array_index(handles, i);
+
+if (h == NULL) {
+g_warn_if_reached();
+continue;
+}
+
+if ((h->watch == watch) && !h->removed) {
+return h;
+}
+}
return NULL;
}
@@ -182,7 +191,7 @@ virt_viewer_events_cleanup_handle(gpointer user_data)
if (data->ff)
(data->ff)(data->opaque);
-free(data);
+g_ptr_array_remove_fast(handles, data);
return FALSE;
}
@@ -199,13 +208,17 @@ virt_viewer_events_remove_handle(int watch)
g_debug("Remove handle %d %d", watch, data->fd);
-if (!data->source)
-return -1;
-
-g_source_remove(data->source);
-data->source = 0;
-data->events = 0;
+if (data->source) {
+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 befure it's fully deleted, that's why we need to
mar it as
+ * 'removed' to prevent reuse
+ */
+data->removed = TRUE;
g_idle_add(virt_viewer_events_cleanup_handle, data);
return 0;
}
@@ -214,6 +227,7 @@ struct virt_viewer_events_timeout
{
int timer;
int interval;
+int removed;
guint source;
virEventTimeoutCallback cb;
void *opaque;
@@ -222,8 +236,7 @@ struct virt_viewer_events_timeout
static int nexttimer = 1;
-static unsigned int ntimeouts = 0;
-static struct virt_viewer_events_timeout **timeouts = NULL;
+GPtrArray *timeouts;
static gboolean
virt_viewer_events_dispatch_timeout(void *opaque)
@@ -243,9 +256,7 @@ virt_viewer_events_add_timeout(int interval,
{
struct virt_viewer_events_timeout *data;
-timeouts = g_realloc(timeouts, sizeof(*timeouts)*(ntimeouts+1));
-data = g_malloc(sizeof(*data));
-memset(data, 0, sizeof(*data));
+data = g_new0(struct virt_viewer_events_timeout, 1);
data->timer = nexttimer++;
data->interval = interval;
@@ -257,7 +268,7 @@ virt_viewer_events_add_timeout(int interval,
virt_viewer_events_dispatch_timeout,
data);
-timeouts[ntimeouts++] = data;
+g_ptr_array_add(timeouts, data);
g_debug("Add timeout %p %d %p %p %d", data, interval, cb, opaque,
data->timer);
@@ -268,10 +279,22 @@ virt_viewer_events_add_timeout(int interval,
static struct virt_viewer_events_timeout *
virt_viewer_events_find_timeout(int timer)
{
-unsigned int i;
-for (i = 0 ; i < ntimeouts ; i++)
-if (timeouts[i]->timer == timer)
-return timeouts[i];
+guint i;
+
+g_return_val_if_fail(timeouts != NULL, NULL);
+
+for (i = 0; i < timeouts->len; i++) {
+struct virt_viewer_events_timeout *t = g_ptr_array_index(timeouts, i);
+
+if (t == NULL) {
+g_warn_if_reached();
+continue;
+}
+
+if ((t-
[virt-tools-list] [virt-viewer][PATCH 3/6] glib-compat: Add g_mutex_new()
Related to: rhbz#1243228 --- src/virt-glib-compat.h | 4 1 file changed, 4 insertions(+) diff --git a/src/virt-glib-compat.h b/src/virt-glib-compat.h index 2c887e9..17c33f0 100644 --- a/src/virt-glib-compat.h +++ b/src/virt-glib-compat.h @@ -76,6 +76,10 @@ GByteArray *g_byte_array_new_take (guint8 *data, gsize len); #define G_SOURCE_REMOVE FALSE #endif +#if GLIB_CHECK_VERSION(2, 31, 0) +#define g_mutex_new() g_new0(GMutex, 1) +#endif + G_END_DECLS #endif // _VIRT_GLIB_COMPAT_H -- 2.4.4 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-viewer][PATCH 1/6] events: Register event using GOnce to avoid multiple initializations
Based on
http://libvirt.org/git/?p=libvirt-glib.git;a=commit;h=8f8d9ce5238dbcbce40aa04ba55b8c55f97c79c0
Related to: rhbz#1243228
---
src/virt-viewer-events.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/virt-viewer-events.c b/src/virt-viewer-events.c
index 3b5a136..6154353 100644
--- a/src/virt-viewer-events.c
+++ b/src/virt-viewer-events.c
@@ -346,14 +346,22 @@ virt_viewer_events_remove_timeout(int timer)
return 0;
}
-
-void virt_viewer_events_register(void) {
+static gpointer event_register_once(gpointer data G_GNUC_UNUSED)
+{
virEventRegisterImpl(virt_viewer_events_add_handle,
virt_viewer_events_update_handle,
virt_viewer_events_remove_handle,
virt_viewer_events_add_timeout,
virt_viewer_events_update_timeout,
virt_viewer_events_remove_timeout);
+
+return NULL;
+}
+
+void virt_viewer_events_register(void) {
+static GOnce once = G_ONCE_INIT;
+
+g_once(&once, event_register_once, NULL);
}
/*
--
2.4.4
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [virt-viewer][PATCH 4/6] events: protect "handles" and "timeouts" agins concurrent access
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
#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
