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

[Christophe Fergeau]

On Mon, Jan 02, 2012 at 03:03:59PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 01/02/2012 02:23 PM, Christophe Fergeau wrote:
> >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>
> 
> <snip>
> 
> >>+static gpointer spice_usb_device_magager_usb_ev_thread(gpointer user_data)
> >
> >s/magager/manager
> >
> 
> Will fix.
> 
> >>+{
> >>+    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_* ?
> >
> 
> That seems like a good idea, will fix.
> 
> >>+
> >>+    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.
> 
> Yes, because the thread must be stopped before we can close the libusb_context,
> which we do on usbdevicemanager destriction, which happens when a session is closed.
> 
> Is it ok for me to push this with the 2 changes agreed upon above?

Yep

Christophe
-------------- 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/cf88e2a1/attachment.pgp>