From 2a31a602663bcca981b5bf094054904e83bfac3f Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Thu, 28 May 2020 16:27:30 +0200 Subject: [PATCH] Get rid of the ugly Win32 threads in group host Running the audio processing and midi dispatcher loops in a regular `std::thread` causes weird memory corruption issues (likely because of calling conventions are not being respected). Luckily this does not cause any issues here, so we can get rid of a lot of ugly glue code and manual memory management. --- src/wine-host/bridges/group.cpp | 57 +++++++++++---------------------- src/wine-host/bridges/group.h | 6 ++-- 2 files changed, 21 insertions(+), 42 deletions(-) diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index 8f9961b9..66ccbae0 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -59,17 +59,6 @@ boost::asio::local::stream_protocol::acceptor create_acceptor_if_inactive( */ std::string create_logger_prefix(const fs::path& socket_path); -uint32_t WINAPI handle_plugin_dispatch_proxy(void* param); - -/** - * CreateThread() is great and allows you to pass a single value to the - * function, so we'll use this to pass both `this` and the parameters to the - * below thread function so it can do its thing. - * - * @relates handle_plugin_dispatch_proxy - */ -using handle_plugin_dispatch_parameters = std::pair; - StdIoCapture::StdIoCapture(boost::asio::io_context& io_context, int file_descriptor) : pipe(io_context), @@ -131,11 +120,17 @@ void GroupBridge::handle_plugin_dispatch(const GroupRequest request) { bridge->handle_dispatch(); logger.log("'" + request.plugin_path + "' has exited"); - // After the plugin has exited, we'll remove this thread's plugin from the - // active plugins. If no active plugins remain, then we'll terminate the - // process. - std::lock_guard lock(active_plugins_mutex); - active_plugins.erase(request); + // After the plugin has exited we'll remove this thread's plugin from the + // active plugins. This is done within the IO context so we can properly + // join the thread again. If no active plugins remain, then we'll terminate + // the process. + boost::asio::post(plugin_context, [&, request]() { + std::lock_guard lock(active_plugins_mutex); + + auto& [thread, bridge] = active_plugins.at(request); + thread.join(); + active_plugins.erase(request); + }); // Defer actually shutting down the process to allow for fast plugin // scanning by allowing plugins to reuse the same group host process @@ -219,13 +214,14 @@ void GroupBridge::accept_requests() { logger.log("Finished initializing '" + request.plugin_path + "'"); - // CreateThread() doesn't support multiple arguments and - // requires manualy memory management. - handle_plugin_dispatch_parameters* thread_params = - new std::pair(this, request); - active_plugins[request] = std::pair( - Win32Thread(handle_plugin_dispatch_proxy, thread_params), - std::move(bridge)); + // Start listening for dispatcher events sent to the plugin's + // socket on another thread. The actual event handling will + // still occur within this IO context. + active_plugins[request] = + std::pair(std::thread([&, request]() { + handle_plugin_dispatch(request); + }), + std::move(bridge)); } catch (const std::runtime_error& error) { logger.log("Error while initializing '" + request.plugin_path + "':"); @@ -258,8 +254,6 @@ void GroupBridge::async_handle_events() { // Handle Win32 messages unless plugins are in the middle of opening // their editor - // TODO: Check if those same weird crashes with Serum are happening - // again with these normal threads if (!should_skip_message_loop()) { std::lock_guard lock(active_plugins_mutex); @@ -360,16 +354,3 @@ std::string create_logger_prefix(const fs::path& socket_path) { return "[" + socket_name + "] "; } - -uint32_t WINAPI handle_plugin_dispatch_proxy(void* param) { - // The Win32 API only allows you to pass a void pointer to threads, so we - // need to use manual memory management. - auto thread_params = static_cast(param); - GroupBridge* instance = thread_params->first; - GroupRequest parameters = thread_params->second; - delete thread_params; - - instance->handle_plugin_dispatch(parameters); - - return 0; -} diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h index bd4fefdb..034f859e 100644 --- a/src/wine-host/bridges/group.h +++ b/src/wine-host/bridges/group.h @@ -127,9 +127,7 @@ class GroupBridge { * Run a plugin's dispatcher and message loop, processing all events on the * main IO context. The plugin will have already been created in * `accept_requests` since it has to be initiated inside of the IO context's - * thread. Called by proxy using `handle_plugin_dispatch_proxy()` in - * `./group.cpp` because the Win32 `CreateThread` API only allows passing a - * single pointer to the function and does not allow lambdas. + * thread. * * Once the plugin has exited, this thread will then remove itself from the * `active_plugins` map. If this causes the vector to become empty, we will @@ -264,7 +262,7 @@ class GroupBridge { * conventions were nto being respected. */ std::unordered_map>> + std::pair>> active_plugins; /** * A mutex to prevent two threads from simultaneously accessing the plugins