Init plugins and handle events on the main thread

Within the plugin IO context.

src/wine-host/bridges/group.cpp

@@ -52,16 +52,16 @@ boost::asio::local::stream_protocol::acceptor create_acceptor_if_inactive(
  */
 std::string create_logger_prefix(const fs::path& socket_path);
 
-uint32_t WINAPI handle_host_plugin_proxy(void* param);
+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_host_plugin_proxy
+ * @relates handle_plugin_dispatch_proxy
  */
-using handle_host_plugin_parameters = std::pair<GroupBridge*, GroupRequest>;
+using handle_plugin_dispatch_parameters = std::pair<GroupBridge*, GroupRequest>;
 
 StdIoCapture::StdIoCapture(boost::asio::io_context& io_context,
                            int file_descriptor)
@@ -91,15 +91,17 @@ StdIoCapture::~StdIoCapture() {
 GroupBridge::GroupBridge(boost::filesystem::path group_socket_path)
     : logger(Logger::create_from_environment(
           create_logger_prefix(group_socket_path))),
-      io_context(),
+      plugin_context(),
       stdio_context(),
       stdout_redirect(stdio_context, STDOUT_FILENO),
       stderr_redirect(stdio_context, STDERR_FILENO),
       group_socket_endpoint(group_socket_path.string()),
       group_socket_acceptor(
-          create_acceptor_if_inactive(io_context, group_socket_endpoint)),
+          create_acceptor_if_inactive(plugin_context, group_socket_endpoint)),
-      shutdown_timer(io_context) {
+      shutdown_timer(plugin_context) {
     // Write this process's original STDOUT and STDERR streams to the logger
+    // TODO: This works for output generated by plugins, but not for debug
+    //       messages generated by wineserver. Is it possible to catch those?
     async_log_pipe_lines(stdout_redirect.pipe, stdout_buffer, "[STDOUT] ");
     async_log_pipe_lines(stderr_redirect.pipe, stderr_buffer, "[STDERR] ");
 
@@ -111,36 +113,30 @@ GroupBridge::~GroupBridge() {
     stdio_handler.join();
 }
 
-void GroupBridge::handle_host_plugin(const GroupRequest request) {
+void GroupBridge::handle_plugin_dispatch(const GroupRequest request) {
     using namespace std::literals::chrono_literals;
 
-    // At this point the `active_plugins` map will already contain a copy of
+    // At this point the `active_plugins` map will already contain the
-    // `parameters` along with this thread's handle
+    // intialized plugin's `Vst2Bridge` instance and this thread's handle
-    // The initialization process for a plugin is identical to that in
+    auto& [thread, bridge] = active_plugins.at(request);
-    // `../individual-host.cpp`
-    logger.log("Received request to host '" + request.plugin_path +
-               "' using socket '" + request.socket_path + "'");
-    try {
-        Vst2Bridge bridge(request.plugin_path, request.socket_path);
-        logger.log("Finished initializing '" + request.plugin_path + "'");
 
-        // Blocks the main thread until the plugin shuts down
+    // Blocks the main thread until the plugin shuts down, handling all events
-        bridge.handle_dispatch();
+    // on the main IO context
+    // TODO: Maybe add a function to each vstbridge that returns whether the
+    //       message loop should be postponed, and pass a function here that
+    //       checks if this is the case for any of the plugins?
+    bridge->handle_dispatch_multi(
+        plugin_context, [&]() { return should_postpone_message_loop(); });
+    logger.log("'" + request.plugin_path + "' has exited");
 
-        logger.log("'" + request.plugin_path + "' has exited");
+    // After the plugin has exited, we'll remove this thread's plugin from the
-    } catch (const std::runtime_error& error) {
+    // active plugins. If no active plugins remain, then we'll terminate the
-        logger.log("Error while initializing '" + request.plugin_path + "':");
+    // process.
-        logger.log(error.what());
-    }
-
-    // After the plugin has exited (either after `effClose()` or because it
-    // failed to initialize), we'll remove this thread's plugin from the active
-    // plugins. If no active plugins remain, then we'll terminate this process.
     std::lock_guard lock(active_plugins_mutex);
     active_plugins.erase(request);
 
-    // Defer shutting down the process to allow for fast plugin scanning by
+    // Defer actually shutting down the process to allow for fast plugin
-    // allowing plugins to reuse the same group host process
+    // scanning by allowing plugins to reuse the same group host process
     shutdown_timer.expires_after(2s);
     shutdown_timer.async_wait([&](const boost::system::error_code& error) {
         // A previous timer gets canceled automatically when another plugin
@@ -153,7 +149,7 @@ void GroupBridge::handle_host_plugin(const GroupRequest request) {
         if (active_plugins.size() == 0) {
             logger.log(
                 "All plugins have exited, shutting down the group process");
-            io_context.stop();
+            plugin_context.stop();
         }
     });
 }
@@ -163,7 +159,12 @@ void GroupBridge::handle_incoming_connections() {
 
     logger.log(
         "Group host is up and running, now accepting incoming connections");
-    io_context.run();
+    plugin_context.run();
+}
+
+bool GroupBridge::should_postpone_message_loop() {
+    // TODO: Iterate over the plugins
+    return false;
 }
 
 void GroupBridge::accept_requests() {
@@ -177,7 +178,7 @@ void GroupBridge::accept_requests() {
                 logger.log("Error while listening for incoming connections:");
                 logger.log(error.message());
 
-                io_context.stop();
+                plugin_context.stop();
             }
 
             // Read the parameters, and then host the plugin in this process
@@ -186,19 +187,36 @@ void GroupBridge::accept_requests() {
             // yabridge plugin will be able to tell if the plugin has caused
             // this process to crash during its initialization to prevent
             // waiting indefinitely on the sockets to be connected to.
-            const auto parameters = read_object<GroupRequest>(socket);
+            const auto request = read_object<GroupRequest>(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.find(parameters) == active_plugins.end());
+            assert(active_plugins.find(request) == active_plugins.end());
 
-            // CreateThread() doesn't support multiple arguments and requires
+            // The plugin has to be initiated on the IO context's thread because
-            // manualy memory management.
+            // this has to be done on the same thread that's handling messages,
-            handle_host_plugin_parameters* thread_params =
+            // and all window messages have to be handled from the same thread.
-                new std::pair<GroupBridge*, GroupRequest>(this, parameters);
+            logger.log("Received request to host '" + request.plugin_path +
-            active_plugins[parameters] =
+                       "' using socket '" + request.socket_path + "'");
-                Win32Thread(handle_host_plugin_proxy, thread_params);
+            try {
+                auto bridge = std::make_unique<Vst2Bridge>(request.plugin_path,
+                                                           request.socket_path);
+                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<GroupBridge*, GroupRequest>(this, request);
+                active_plugins[request] = std::pair(
+                    Win32Thread(handle_plugin_dispatch_proxy, thread_params),
+                    std::move(bridge));
+            } catch (const std::runtime_error& error) {
+                logger.log("Error while initializing '" + request.plugin_path +
+                           "':");
+                logger.log(error.what());
+            }
 
             accept_requests();
         });
@@ -284,15 +302,15 @@ std::string create_logger_prefix(const fs::path& socket_path) {
     return "[" + socket_name + "] ";
 }
 
-uint32_t WINAPI handle_host_plugin_proxy(void* param) {
+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<handle_host_plugin_parameters*>(param);
+    auto thread_params = static_cast<handle_plugin_dispatch_parameters*>(param);
     GroupBridge* instance = thread_params->first;
     GroupRequest parameters = thread_params->second;
     delete thread_params;
 
-    instance->handle_host_plugin(parameters);
+    instance->handle_plugin_dispatch(parameters);
 
     return 0;
 }

src/wine-host/bridges/group.h

@@ -124,12 +124,12 @@ class GroupBridge {
     GroupBridge& operator=(const GroupBridge&) = delete;
 
     /**
-     * Host a new plugin within this process. Called by proxy using
+     * Run a plugin's dispatcher and message loop, processing all events on the
-     * `handle_host_plugin_proxy()` in `./group.cpp` because the Win32
+     * main IO context. The plugin will have already been created in
-     * `CreateThread` API only allows passing a single pointer to the function
+     * `accept_requests` since it has to be initiated inside of the IO context's
-     * and does not allow lambdas. Because we don't have access to our own
+     * thread. Called by proxy using `handle_plugin_dispatch_proxy()` in
-     * thread handle, the thread that's listening on the group socket will have
+     * `./group.cpp` because the Win32 `CreateThread` API only allows passing a
-     * already added this thread to the `active_plugins` map.
+     * single pointer to the function and does not allow lambdas.
      *
      * 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
@@ -145,7 +145,7 @@ class GroupBridge {
      *   then the process will never exit on its own. This should not happen
      *   though.
      */
-    void handle_host_plugin(const GroupRequest request);
+    void handle_plugin_dispatch(const GroupRequest request);
 
     /**
      * Listen for new requests to spawn plugins within this process and handle
@@ -153,15 +153,33 @@ class GroupBridge {
      */
     void handle_incoming_connections();
 
+    /**
+     * Returns true if the message loop should not be run at this time. This is
+     * necessary because hosts will always call either `effEditOpen()` and then
+     * `effEditGetRect()` or the other way around. If the message loop is
+     * handled in between these two actions, then some plugins will either
+     * freeze or sometimes outright crash. Because every plugin has to be run
+     * from the same thread, this is a simple way to synchronize blocking the
+     * mesage loop between the different plugin instances.
+     */
+    bool should_postpone_message_loop();
+
    private:
     /**
      * Listen on the group socket for incoming requests to host a new plugin
-     * within this group process. This will asynchronously listen on the socket,
+     * within this group process. This will read a `GoupRequest` object
-     * and for any connection made it will retrieve a `GroupRequest` object
+     * containing information about the plugin, reply with this process's PID so
-     * containing information about the plugin to host and then spawn a new
+     * the yabridge instance can tell if the plugin crashed during
-     * thread to start hosting that plugin.
+     * initialization, and it will then try to initialize the plugin. After
+     * intialization the plugin handling will be handed over to a new thread
+     * running `handle_plugin_dispatch()`. Because of the way the Win32 API
+     * works, all plugins have to be initialized from the same thread, and all
+     * event handling and message loop interaction also has to be done from that
+     * thread, which is why we initialize the plugin here and use the
+     * `handle_dispatch_multi()` function to run events within the same
+     * `plugin_context`.
      *
-     * @see handle_host_plugin
+     * @see handle_plugin_dispatch
      */
     void accept_requests();
 
@@ -186,7 +204,12 @@ class GroupBridge {
      */
     Logger logger;
 
-    boost::asio::io_context io_context;
+    /**
+     * The IO context that connections will be accepted on, and that any plugin
+     * operations that may involve the Win32 mesasge loop (e.g. initialization
+     * and most `AEffect::dispatcher()` calls) should be run on.
+     */
+    boost::asio::io_context plugin_context;
     /**
      * A seperate IO context that handles the STDIO redirect through
      * `StdIoCapture`. This is seperated the `plugin_context` above so that
@@ -223,16 +246,23 @@ class GroupBridge {
     boost::asio::local::stream_protocol::acceptor group_socket_acceptor;
 
     /**
-     * A map of threads that are currently hosting a plugin within this process.
+     * A map of threads that are currently hosting a plugin within this process
-     * After a plugin has exited or its initialization has failed, the thread
+     * along with their plugin instance. After a plugin has exited or its
-     * handling it will remove itself from this map. This is to keep track of
+     * initialization has failed, the thread handling it will remove itself from
-     * the amount of plugins currently running with their associated thread
+     * this map. This is to keep track of the amount of plugins currently
-     * handles.
+     * running with their associated thread handles.
+     *
+     * TODO: Check again if we can just use std::thread here instead, that would
+     *       make everything much simpler. `std::thread` was a problem with
+     *       gdiplus in the past as Serum would randomly crash because calling
+     *       conventions were nto being respected.
      */
-    std::unordered_map<GroupRequest, Win32Thread> active_plugins;
+    std::unordered_map<GroupRequest,
+                       std::pair<Win32Thread, std::unique_ptr<Vst2Bridge>>>
+        active_plugins;
     /**
      * A mutex to prevent two threads from simultaneously accessing the plugins
-     * map, and also to prevent `handle_host_plugin()` from terminating the
+     * map, and also to prevent `handle_plugin_dispatch()` from terminating the
      * process because it thinks there are no active plugins left just as a new
      * plugin is being spawned.
      */
@@ -243,7 +273,7 @@ class GroupBridge {
      * scanning without having to start a new group host process for each
      * plugin.
      *
-     * @see handle_host_plugin
+     * @see handle_plugin_dispatch
      */
     boost::asio::steady_timer shutdown_timer;
 };