Skip to content

Commit 9e478b5

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 9e478b5

File tree

2 files changed

+21
-20
lines changed

2 files changed

+21
-20
lines changed

plugins/usbdmx/AsyncPluginImpl.cpp

Lines changed: 20 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,26 @@ 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 must be run within the main thread. The Device destructor below may
352+
* cause libusb_close() to be called, which would deadlock if the hotplug
353+
* event thread were to wait for it, say in AsyncPluginImpl::DeviceEvent().
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;
362+
} else {
363+
/* This case can be legitimate when the widget setup through the
364+
* widget factory is delayed and an unplug event happens before
365+
* that is completed. */
366+
OLA_DEBUG << "ola_device was NULL at shutdown";
366367
}
368+
state->DeleteWidget();
367369
}
368370
} // namespace usbdmx
369371
} // 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)