diff --git a/CHANGELOG.md b/CHANGELOG.md index f8fb80c9..c21edff8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- Fixed some plugins, notably the _Spitfire Audio_ plugins, from causing a + deadlock when using plugin groups in _REAPER_. Even though this did not seem + to cause any issues in other hosts, the race condition that caused this issue + could also occur elsewhere. + ## [2.2.0] - 2020-12-11 ### Added diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index 8b546cb5..8cb528f1 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -105,17 +105,8 @@ GroupBridge::~GroupBridge() { stdio_context.stop(); } -void GroupBridge::handle_plugin_dispatch(size_t plugin_id) { - // At this point the `active_plugins` map will already contain the - // intialized plugin's `Vst2Bridge` instance and this thread's handle - Vst2Bridge* bridge; - { - std::lock_guard lock(active_plugins_mutex); - bridge = active_plugins[plugin_id].second.get(); - } - - // Blocks this thread until the plugin shuts down, handling all events on - // the main IO context +void GroupBridge::handle_plugin_dispatch(size_t plugin_id, Vst2Bridge* bridge) { + // Blocks this thread until the plugin shuts down bridge->handle_dispatch(); logger.log("'" + bridge->vst_plugin_path.string() + "' has exited"); @@ -202,16 +193,22 @@ void GroupBridge::accept_requests() { "'"); // Start listening for dispatcher events sent to the plugin's - // socket on another thread. The actual event handling will - // still be posted to this IO context so that every plugin's - // primary event handling happens on the main thread. Since this - // is only used within this context we don't need any locks. + // socket on another thread. Parts of the actual event handling + // will still be posted to this IO context so that any events + // that potentially interact with the Win32 message loop are + // handled from the main thread. We also pass a raw pointer to + // the plugin so we don't have to immediately look the instance + // up in the map again, as this would require us to immediately + // lock the map again. This could otherwise result in a deadlock + // when using the Spitfire plugins, as they will block the + // message loop until `effOpen()` has been called and thus + // prevent this lock from happening. const size_t plugin_id = next_plugin_id.fetch_add(1); - active_plugins[plugin_id] = - std::pair(Win32Thread([this, plugin_id]() { - handle_plugin_dispatch(plugin_id); - }), - std::move(bridge)); + active_plugins[plugin_id] = std::pair( + Win32Thread([this, plugin_id, plugin_ptr = bridge.get()]() { + handle_plugin_dispatch(plugin_id, plugin_ptr); + }), + std::move(bridge)); } catch (const std::runtime_error& error) { logger.log("Error while initializing '" + request.plugin_path + "':"); diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h index 6dfccee7..5a849b56 100644 --- a/src/wine-host/bridges/group.h +++ b/src/wine-host/bridges/group.h @@ -139,15 +139,14 @@ class GroupBridge { * This check is delayed by a few seconds to prevent having to constantly * restart the group process during plugin scanning. * - * @param plugin_id The ID of this plugin in the `active_plugins` map. The - * thread can fetch the plugin's `Vst2Bridge` instance from that map using - * this identifier. + * @param plugin_id The ID of this plugin in the `active_plugins` map. Used + * to unload the plugin and join this thread again after the plugin exits. * * @note In the case that the process starts but no plugin gets initiated, * then the process will never exit on its own. This should not happen * though. */ - void handle_plugin_dispatch(size_t plugin_id); + void handle_plugin_dispatch(size_t plugin_id, Vst2Bridge* bridge); /** * Listen for new requests to spawn plugins within this process and handle