[Spice-devel] [spice-gtk 3/3] usb-device-manager: Fix USB event thread leak

[Christophe Fergeau]

When using USB redirection, it's fairly easy to leak the thread handling
USB events, which will eventually cause problems in long lived apps.
In particular, in virt-manager, one can:
- start a VM
- connect to it with SPICE
- open the USB redirection window
- redirect a device
- close the SPICE window
-> the SpiceUsbDeviceManager instance will be destroyed (including the
USB context it owns), but the associated event thread will keep running.
Since it's running a loop blocking on libusb_handle_events(priv->context),
the loop will eventually try to use the USB context we just destroyed
causing a crash.

We can get in this situation when redirecting a USB device because we
will call spice_usb_device_manager_start_event_listening() in
spice_usbredir_channel_open_device(). The matching
spice_usb_device_manager_stop_event_listening() call is supposed to
happen in spice_usbredir_channel_disconnect_device(), however by the
time it's called in the scenario described above, the session associated
with the channel will already have been set to NULL in
spice_session_channel_destroy().

Since the USB event thread has to be stopped when we destroy the
associated SpiceUsbDeviceManager, spice_usb_device_manager_dispose()
should force event_thread_run to FALSE rather than calling
spice_usb_device_manager_stop_event_listening() which will only set it
to FALSE if this was the only user of the event thread.

This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1217202
(virt-manager) and most
likely https://bugzilla.redhat.com/show_bug.cgi?id=1337007 (gnome-boxes)
as well.
---
 src/usb-device-manager.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
index 1912b62..2b10be2 100644
--- a/src/usb-device-manager.c
+++ b/src/usb-device-manager.c
@@ -374,7 +374,9 @@ static void spice_usb_device_manager_dispose(GObject *gobject)
 
 #ifdef USE_LIBUSB_HOTPLUG
     if (priv->hp_handle) {
-        spice_usb_device_manager_stop_event_listening(self);
+        /* Force termination of the event thread even if there were some mismatched
+         * spice_usb_device_manager_{start,stop}_event_listening calls */
+        g_atomic_int_set(&priv->event_thread_run, FALSE);
         /* This also wakes up the libusb_handle_events() in the event_thread */
         libusb_hotplug_deregister_callback(priv->context, priv->hp_handle);
         priv->hp_handle = 0;
-- 
2.7.4