[virt-tools-list] [virt-viewer v2 01/13] events: ensure event callbacks are threadsafe

[Fabiano Fidêncio]

On Wed, Jul 22, 2015 at 10:04 AM, Fabiano Fidêncio <fidencio at redhat.com> wrote:
> Take a global lock whenever changing any event callbacks to ensure
> thread safety.
>
> Based on commit f1fe67da2dac6a249f796535b8dbd155d5741ad7 from
> libvirt-glib.
> Original author: Daniel P. Berrange <berrange at redhat.com>
>
> Related to: rhbz#1243228
> ---
>  src/virt-viewer-events.c | 84 +++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 66 insertions(+), 18 deletions(-)
>
> diff --git a/src/virt-viewer-events.c b/src/virt-viewer-events.c
> index 3b5a136..b0a84ba 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"
> +

This include should be part of the patch 5. It's already fixed locally.

> +static GMutex *eventlock = NULL;
>
>  struct virt_viewer_events_handle
>  {
> @@ -85,6 +88,9 @@ int virt_viewer_events_add_handle(int fd,
>  {
>      struct virt_viewer_events_handle *data;
>      GIOCondition cond = 0;
> +    int ret;
> +
> +    g_mutex_lock(eventlock);
>
>      handles = g_realloc(handles, sizeof(*handles)*(nhandles+1));
>      data = g_malloc(sizeof(*data));
> @@ -117,7 +123,11 @@ int virt_viewer_events_add_handle(int fd,
>
>      handles[nhandles++] = data;
>
> -    return data->watch;
> +    ret = data->watch;
> +
> +    g_mutex_unlock(eventlock);
> +
> +    return ret;
>  }
>
>  static struct virt_viewer_events_handle *
> @@ -135,17 +145,21 @@ 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);
> +
> +    data = virt_viewer_events_find_handle(watch);
>
>      if (!data) {
>          g_debug("Update for missing handle watch %d", watch);
> -        return;
> +        goto cleanup;
>      }
>
>      if (events) {
>          GIOCondition cond = 0;
>          if (events == data->events)
> -            return;
> +            goto cleanup;
>
>          if (data->source)
>              g_source_remove(data->source);
> @@ -162,12 +176,15 @@ virt_viewer_events_update_handle(int watch,
>          data->events = events;
>      } else {
>          if (!data->source)
> -            return;
> +            goto cleanup;
>
>          g_source_remove(data->source);
>          data->source = 0;
>          data->events = 0;
>      }
> +
> +cleanup:
> +    g_mutex_unlock(eventlock);
>  }
>
>
> @@ -190,24 +207,33 @@ 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;
> +
> +    g_mutex_lock(eventlock);
> +
> +    data = virt_viewer_events_find_handle(watch);
>
>      if (!data) {
>          g_debug("Remove of missing watch %d", watch);
> -        return -1;
> +        goto cleanup;
>      }
>
>      g_debug("Remove handle %d %d", watch, data->fd);
>
>      if (!data->source)
> -        return -1;
> +        goto cleanup;
>
>      g_source_remove(data->source);
>      data->source = 0;
>      data->events = 0;
>
>      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
> @@ -242,6 +268,9 @@ virt_viewer_events_add_timeout(int interval,
>                                 virFreeCallback ff)
>  {
>      struct virt_viewer_events_timeout *data;
> +    int ret;
> +
> +    g_mutex_lock(eventlock);
>
>      timeouts = g_realloc(timeouts, sizeof(*timeouts)*(ntimeouts+1));
>      data = g_malloc(sizeof(*data));
> @@ -261,7 +290,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;
>  }
>
>
> @@ -281,18 +314,21 @@ 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);
> +
> +    data = virt_viewer_events_find_timeout(timer);
>      if (!data) {
>          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;
> +            goto cleanup;
>
>          data->interval = interval;
>          data->source = g_timeout_add(data->interval,
> @@ -300,11 +336,14 @@ virt_viewer_events_update_timeout(int timer,
>                                       data);
>      } else {
>          if (!data->source)
> -            return;
> +            goto cleanup;
>
>          g_source_remove(data->source);
>          data->source = 0;
>      }
> +
> +cleanup:
> +    g_mutex_unlock(eventlock);
>  }
>
>
> @@ -327,27 +366,36 @@ 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);
> +
> +    data = virt_viewer_events_find_timeout(timer);
>      if (!data) {
>          g_debug("Remove of missing timer %d", timer);
> -        return -1;
> +        goto cleanup;
>      }
>
>      g_debug("Remove timeout %p %d", data, timer);
>
>      if (!data->source)
> -        return -1;
> +        goto cleanup;
>
>      g_source_remove(data->source);
>      data->source = 0;
>
>      g_idle_add(virt_viewer_events_cleanup_timeout, data);
> -    return 0;
> +    ret = 0;
> +
> +cleanup:
> +    g_mutex_unlock(eventlock);
> +    return ret;
>  }
>
>
>  void virt_viewer_events_register(void) {
> +    eventlock = g_mutex_new();
>      virEventRegisterImpl(virt_viewer_events_add_handle,
>                           virt_viewer_events_update_handle,
>                           virt_viewer_events_remove_handle,
> --
> 2.4.4
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list



-- 
Fabiano Fidêncio