Hi,
On Thu, Jul 11, 2019 at 09:42:02AM -0400, Frediano Ziglio wrote:
> >
> > Hi,
> >
> > On Thu, Jul 11, 2019 at 02:00:54PM +0100, Frediano Ziglio wrote:
> > > Attempt to better interrupt event handling loop.
> > > If the thread handling events is stuck waiting events or handling an
> > > event try to interrupt before joining the thread.
> >
> > Do you have a UI stuck or something without this patch?
> >
>
> Good question. Never happened, however:
> - I had some experience in the past with libusb and multi-threading
> and I know this call is useful;
> - for Unix (but not Windows) there's some lines above the comment
> /* Force termination of the event thread even if there were some
> * mismatched spice_usb_device_manager_{start,stop}_event_listening
> * calls. Otherwise, the usb event thread will be leaked, and will
> * try to use the libusb context we destroy in finalize(), which would
> * cause a crash */
> so calling that function on all (Unix and Windows) will help too.
>
>
> Maybe adding in commit message:
>
> "For Unix code in spice_usb_device_manager_dispose will try to
> force some thread exit but this is not done for Windows
> so calling libusb_interrupt_event_handler will help.
> Code is not in a hot path so won't change the execution time."Sounds reasonable to me, with that Acked-by: Victor Toso <[email protected]> > > > > Signed-off-by: Frediano Ziglio <[email protected]> > > > --- > > > src/usb-backend.c | 7 +++++++ > > > src/usb-backend.h | 1 + > > > src/usb-device-manager.c | 4 ++++ > > > 3 files changed, 12 insertions(+) > > > > > > diff --git a/src/usb-backend.c b/src/usb-backend.c > > > index 48a62cd1..a2c502da 100644 > > > --- a/src/usb-backend.c > > > +++ b/src/usb-backend.c > > > @@ -230,6 +230,13 @@ gboolean > > > spice_usb_backend_handle_events(SpiceUsbBackend *be) > > > return ok; > > > } > > > > > > +void spice_usb_backend_interrupt_event_handler(SpiceUsbBackend *be) > > > +{ > > > + if (be->libusb_context) { > > > + libusb_interrupt_event_handler(be->libusb_context); > > > + } > > > +} > > > + > > > static int LIBUSB_CALL hotplug_callback(libusb_context *ctx, > > > libusb_device *device, > > > libusb_hotplug_event event, > > > diff --git a/src/usb-backend.h b/src/usb-backend.h > > > index 9f2a97a6..cbb73c22 100644 > > > --- a/src/usb-backend.h > > > +++ b/src/usb-backend.h > > > @@ -65,6 +65,7 @@ after it finishes list processing > > > SpiceUsbBackendDevice **spice_usb_backend_get_device_list(SpiceUsbBackend > > > *backend); > > > void spice_usb_backend_free_device_list(SpiceUsbBackendDevice **devlist); > > > gboolean spice_usb_backend_handle_events(SpiceUsbBackend *be); > > > +void spice_usb_backend_interrupt_event_handler(SpiceUsbBackend *be); > > > gboolean spice_usb_backend_register_hotplug(SpiceUsbBackend *be, > > > void *user_data, > > > usb_hot_plug_callback proc); > > > diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c > > > index 4960667e..0d12432f 100644 > > > --- a/src/usb-device-manager.c > > > +++ b/src/usb-device-manager.c > > > @@ -328,6 +328,8 @@ static void spice_usb_device_manager_dispose(GObject > > > *gobject) > > > #endif > > > if (priv->event_thread) { > > > g_warn_if_fail(g_atomic_int_get(&priv->event_thread_run) == > > > FALSE); > > > + g_atomic_int_set(&priv->event_thread_run, FALSE); > > > + spice_usb_backend_interrupt_event_handler(priv->context); > > > g_thread_join(priv->event_thread); > > > priv->event_thread = NULL; > > > } > > > @@ -988,6 +990,8 @@ gboolean > > > spice_usb_device_manager_start_event_listening( > > > 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_atomic_int_set(&priv->event_thread_run, FALSE); > > > + spice_usb_backend_interrupt_event_handler(priv->context); > > > g_thread_join(priv->event_thread); > > > priv->event_thread = NULL; > > > } > > Frediano
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/spice-devel
