From d82f8463d9a1cce1e6f58ba071ad81414befa3fb Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 26 Oct 2020 11:51:13 +0100 Subject: [PATCH] Use simple numerical IDs for plugins in groups We'll be using a similar approach to identify threads for event handlers. --- src/common/serialization.cpp | 5 ----- src/common/serialization.h | 2 -- src/wine-host/bridges/group.cpp | 23 +++++++++++------------ src/wine-host/bridges/group.h | 31 ++++++++++++++++++++----------- src/wine-host/bridges/vst2.cpp | 3 ++- src/wine-host/bridges/vst2.h | 5 +++++ 6 files changed, 38 insertions(+), 31 deletions(-) diff --git a/src/common/serialization.cpp b/src/common/serialization.cpp index d49d480d..cb9662d3 100644 --- a/src/common/serialization.cpp +++ b/src/common/serialization.cpp @@ -108,8 +108,3 @@ AEffect& update_aeffect(AEffect& plugin, const AEffect& updated_plugin) { return plugin; } - -bool GroupRequest::operator==(const GroupRequest& rhs) const { - return plugin_path == rhs.plugin_path && - endpoint_base_dir == rhs.endpoint_base_dir; -} diff --git a/src/common/serialization.h b/src/common/serialization.h index c53612a5..18829b46 100644 --- a/src/common/serialization.h +++ b/src/common/serialization.h @@ -600,8 +600,6 @@ struct GroupRequest { std::string plugin_path; std::string endpoint_base_dir; - bool operator==(const GroupRequest& rhs) const; - template void serialize(S& s) { s.text1b(plugin_path, 4096); diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index b50fdbe4..1b0e9e49 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -111,15 +111,15 @@ GroupBridge::~GroupBridge() { stdio_context.stop(); } -void GroupBridge::handle_plugin_dispatch(const GroupRequest request) { +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 - auto& [thread, bridge] = active_plugins.at(request); + auto& [thread, bridge] = active_plugins[plugin_id]; // Blocks this thread until the plugin shuts down, handling all events on // the main IO context bridge->handle_dispatch(); - logger.log("'" + request.plugin_path + "' has exited"); + logger.log("'" + bridge->vst_plugin_path.string() + "' has exited"); // After the plugin has exited we'll remove this thread's plugin from the // active plugins. This is done within the IO context because the call to @@ -127,11 +127,11 @@ void GroupBridge::handle_plugin_dispatch(const GroupRequest request) { // potentially corrupt our heap. This way we can also properly join the // thread again. If no active plugins remain, then we'll terminate the // process. - boost::asio::post(plugin_context, [&, request]() { + boost::asio::post(plugin_context, [this, plugin_id]() { std::lock_guard lock(active_plugins_mutex); // The join is implicit because we're using std::jthread - active_plugins.erase(request); + active_plugins.erase(plugin_id); }); // Defer actually shutting down the process to allow for fast plugin @@ -201,10 +201,6 @@ void GroupBridge::accept_requests() { const auto request = read_object(socket); write_object(socket, GroupResponse{boost::this_process::get_id()}); - // Collisions in the generated socket names should be very rare, but - // it could in theory happen - assert(!active_plugins.contains(request)); - // The plugin has to be initiated on the IO context's thread because // this has to be done on the same thread that's handling messages, // and all window messages have to be handled from the same thread. @@ -220,10 +216,13 @@ void GroupBridge::accept_requests() { // 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] = + // 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. + const size_t plugin_id = next_plugin_id.fetch_add(1); + active_plugins[plugin_id] = std::pair(std::jthread([&, request]() { - handle_plugin_dispatch(request); + handle_plugin_dispatch(plugin_id); }), std::move(bridge)); } catch (const std::runtime_error& error) { diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h index c926cd9c..cf7690e8 100644 --- a/src/wine-host/bridges/group.h +++ b/src/wine-host/bridges/group.h @@ -22,6 +22,7 @@ #include #include +#include #include #include "vst2.h" @@ -132,21 +133,21 @@ class GroupBridge { * `accept_requests` since it has to be initiated inside of the IO context's * 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 - * terminate this process. This check will be delayed by a few seconds to - * prevent having to constantly restart the group process during plugin - * scanning. + * Once the plugin has exited, this thread will then be joined to the main + * thread and removed from the `active_plugins` from the main IO context. If + * this causes the vector to become empty, we will terminate this process. + * This check is delayed by a few seconds to prevent having to constantly + * restart the group process during plugin scanning. * - * @param request Information about the plugin to launch, i.e. the path to - * the plugin and the path of the socket endpoint that will be used for - * communication. + * @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. * * @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(const GroupRequest request); + void handle_plugin_dispatch(size_t plugin_id); /** * Listen for new requests to spawn plugins within this process and handle @@ -257,11 +258,19 @@ class GroupBridge { * along with their plugin instance. After a plugin has exited or its * initialization has failed, the thread handling it will remove itself from * this map. This is to keep track of the amount of plugins currently - * running with their associated thread handles. + * running with their associated thread handles. The key that identifies the + * thread and plugin is a unique plugin ID obtained by doing a fetch-and-add + * on `next_plugin_id`. */ - std::unordered_map>> active_plugins; + /** + * A counter for the next unique plugin ID. When hosting a new plugin we'll + * do a fetch-and-add to ensure that every thread gets its own unique + * identifier. + */ + std::atomic_size_t next_plugin_id; /** * A mutex to prevent two threads from simultaneously accessing the plugins * map, and also to prevent `handle_plugin_dispatch()` from terminating the diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index 93adef5b..846ae6f8 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -67,7 +67,8 @@ Vst2Bridge& get_bridge_instance(const AEffect* plugin) { Vst2Bridge::Vst2Bridge(boost::asio::io_context& main_context, std::string plugin_dll_path, std::string endpoint_base_dir) - : io_context(main_context), + : vst_plugin_path(plugin_dll_path), + io_context(main_context), plugin_handle(LoadLibrary(plugin_dll_path.c_str()), FreeLibrary), sockets(io_context, endpoint_base_dir, false) { // Got to love these C APIs diff --git a/src/wine-host/bridges/vst2.h b/src/wine-host/bridges/vst2.h index 38adc306..0a0ada14 100644 --- a/src/wine-host/bridges/vst2.h +++ b/src/wine-host/bridges/vst2.h @@ -150,6 +150,11 @@ class Vst2Bridge { */ std::optional time_info; + /** + * The path to the .dll being loaded in the Wine VST host. + */ + const boost::filesystem::path vst_plugin_path; + private: /** * A wrapper around `plugin->dispatcher` that handles the opening and