Skip to content

Commit e9d0e3f

Browse files
committed
usbdmx: Move widget deletion to main thread
Currently when a usb device is remove the libusb event thread, though the hotplug handler, will cause the widget's Device to be deleted in the main thread and wait for this to complete in the libusb thread. If any of the destructors triggered by device deletion causes libusb_close to be called this causes a deadlock as libusb_close will try to take over handling of events in the main thread while the libusb thread is waiting for libusb_close, via the destructors, to finish. Just move both widget and device deletion to the main thread to prevent this scenario.
1 parent a95a6f7 commit e9d0e3f

File tree

2 files changed

+16
-20
lines changed

2 files changed

+16
-20
lines changed

plugins/usbdmx/AsyncPluginImpl.cpp

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -264,16 +264,9 @@ void AsyncPluginImpl::DeviceEvent(HotplugAgent::EventType event,
264264
// Sunlite plugin, if we make the f/w load async we'll need to let the
265265
// factory cancel the load.
266266

267-
// Unregister & delete the device in the main thread.
268-
if (state->ola_device) {
269-
Future<void> f;
270-
m_plugin_adaptor->Execute(
271-
NewSingleCallback(this, &AsyncPluginImpl::ShutdownDevice,
272-
state->ola_device, &f));
273-
f.Get();
274-
state->ola_device = NULL;
275-
}
276-
state->DeleteWidget();
267+
// Unregister & delete the device and widget in the main thread.
268+
m_plugin_adaptor->Execute(
269+
NewSingleCallback(this, &AsyncPluginImpl::ShutdownDeviceState, state));
277270
}
278271
}
279272

@@ -353,17 +346,21 @@ bool AsyncPluginImpl::StartAndRegisterDevice(Widget *widget, Device *device) {
353346

354347
/*
355348
* @brief Signal widget removal.
356-
* @param device_id The device id to remove.
349+
* @param state The DeviceState owning the widget to be removed.
357350
*
358-
* This is run within the main thread.
351+
* This is run within the main thread and it must. The Device destructor
352+
* below may cause libusb_close() to be called, which will deadlock if the
353+
* hotplug event thread waits for it.
359354
*/
360-
void AsyncPluginImpl::ShutdownDevice(Device *device, Future<void> *f) {
361-
m_plugin_adaptor->UnregisterDevice(device);
362-
device->Stop();
363-
delete device;
364-
if (f) {
365-
f->Set();
355+
void AsyncPluginImpl::ShutdownDeviceState(DeviceState *state) {
356+
if (state->ola_device) {
357+
Device *device = state->ola_device;
358+
m_plugin_adaptor->UnregisterDevice(device);
359+
device->Stop();
360+
delete device;
361+
state->ola_device = NULL;
366362
}
363+
state->DeleteWidget();
367364
}
368365
} // namespace usbdmx
369366
} // namespace plugin

plugins/usbdmx/AsyncPluginImpl.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
#include "libs/usb/HotplugAgent.h"
3838

3939
#include "ola/base/Macro.h"
40-
#include "ola/thread/Future.h"
4140
#include "olad/Preferences.h"
4241
#include "plugins/usbdmx/PluginImplInterface.h"
4342
#include "plugins/usbdmx/SynchronizedWidgetObserver.h"
@@ -111,7 +110,7 @@ class AsyncPluginImpl: public PluginImplInterface, public WidgetObserver {
111110
template <typename Widget>
112111
bool StartAndRegisterDevice(Widget *widget, Device *device);
113112

114-
void ShutdownDevice(Device *device, ola::thread::Future<void> *f);
113+
void ShutdownDeviceState(DeviceState *state);
115114

116115
DISALLOW_COPY_AND_ASSIGN(AsyncPluginImpl);
117116
};

0 commit comments

Comments
 (0)