[Spice-devel] [PATCH spice-gtk 07/10] usbredir: Handle usb events from a thread

[Christophe Fergeau]

On Mon, Dec 19, 2011 at 12:24:40PM +0100, Hans de Goede wrote:
> This solves various latency issues with USB handling.
> 
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  gtk/channel-usbredir.c   |    5 +++++
>  gtk/usb-device-manager.c |   45 ++++++++++++++++++++++++++++++++-------------
>  2 files changed, 37 insertions(+), 13 deletions(-)
> 
> diff --git a/gtk/channel-usbredir.c b/gtk/channel-usbredir.c
> index dfd1655..e14088a 100644
> --- a/gtk/channel-usbredir.c
> +++ b/gtk/channel-usbredir.c
> @@ -352,6 +352,11 @@ void spice_usbredir_channel_disconnect(SpiceUsbredirChannel *channel)
>      case STATE_CONNECTING:
>      case STATE_CONNECTED:
>          spice_channel_disconnect(SPICE_CHANNEL(channel), SPICE_CHANNEL_NONE);
> +        /*
> +         * This sets the usb event thread run condition to FALSE, therefor
> +         * it must be done before usbredirhost_close, as usbredirhost_close
> +         * will interrupt the libusb_handle_events call in the thread.
> +         */
>          spice_usb_device_manager_stop_event_listening(
>              spice_usb_device_manager_get(
>                  spice_channel_get_session(SPICE_CHANNEL(channel)),
> diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c
> index 6fba6f6..44ed84d 100644
> --- a/gtk/usb-device-manager.c
> +++ b/gtk/usb-device-manager.c
> @@ -26,8 +26,8 @@
>  #include "glib-compat.h"
>  
>  #ifdef USE_USBREDIR
> -#include <gusb/gusb-source.h>
>  #include <gusb/gusb-device-list.h>
> +#include <gusb/gusb-context-private.h>
>  #include "channel-usbredir-priv.h"
>  #endif
>  
> @@ -91,8 +91,9 @@ struct _SpiceUsbDeviceManagerPrivate {
>  #ifdef USE_USBREDIR
>      GUsbContext *context;
>      GUsbDeviceList *devlist;
> -    GUsbSource *source;
>      int event_listeners;
> +    GThread *event_thread;
> +    gboolean event_thread_run;
>  #endif
>      GPtrArray *devices;
>      GPtrArray *channels;
> @@ -204,6 +205,9 @@ static void spice_usb_device_manager_finalize(GObject *gobject)
>      SpiceUsbDeviceManagerPrivate *priv = self->priv;
>  
>  #ifdef USE_USBREDIR
> +    if (priv->event_thread)
> +        g_thread_join(priv->event_thread);
> +
>      if (priv->devlist) {
>          g_object_unref(priv->devlist);
>          g_object_unref(priv->context);
> @@ -480,6 +484,18 @@ static void spice_usb_device_manager_channel_connect_cb(
>  /* ------------------------------------------------------------------ */
>  /* private api                                                        */
>  
> +static gpointer spice_usb_device_magager_usb_ev_thread(gpointer user_data)

s/magager/manager

> +{
> +    SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(user_data);
> +    SpiceUsbDeviceManagerPrivate *priv = self->priv;
> +
> +    while (priv->event_thread_run) {
> +        libusb_handle_events(_g_usb_context_get_context(priv->context));
> +    }

Maybe we should log something when libusb_handle_events returns
LIBUSB_ERROR_* ?

> +
> +    return NULL;
> +}
> +
>  gboolean spice_usb_device_manager_start_event_listening(
>      SpiceUsbDeviceManager *self, GError **err)
>  {
> @@ -491,10 +507,18 @@ gboolean spice_usb_device_manager_start_event_listening(
>      if (priv->event_listeners > 1)
>          return TRUE;
>  
> -    g_return_val_if_fail(priv->source == NULL, FALSE);
> -
> -    priv->source = g_usb_source_new(priv->main_context, priv->context, err);
> -    return priv->source != NULL;
> +    /* We don't join the thread when we stop event listening, as the
> +       libusb_handle_events call in the thread won't exit until the
> +       libusb_close call for the device is made from usbredirhost_close. */
> +    if (priv->event_thread) {
> +         g_thread_join(priv->event_thread);
> +         priv->event_thread = NULL;
> +    }
> +    priv->event_thread_run = TRUE;
> +    priv->event_thread = g_thread_create(
> +                             spice_usb_device_magager_usb_ev_thread,
> +                             self, TRUE, err);

Do we need to join threads at all? Not needing to call g_thread_join would
make the code simpler. Ie the 3rd arg could be FALSE here.

Christophe

> +    return priv->event_thread != NULL;
>  }
>  
>  void spice_usb_device_manager_stop_event_listening(
> @@ -505,13 +529,8 @@ void spice_usb_device_manager_stop_event_listening(
>      g_return_if_fail(priv->event_listeners > 0);
>  
>      priv->event_listeners--;
> -    if (priv->event_listeners != 0)
> -        return;
> -
> -    g_return_if_fail(priv->source != NULL);
> -
> -    g_usb_source_destroy(priv->source);
> -    priv->source = NULL;
> +    if (priv->event_listeners == 0)
> +        priv->event_thread_run = FALSE;
>  }
>  #endif
>  
> -- 
> 1.7.7.4
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20120102/1d0a506a/attachment.pgp>