💥 Encapsulate and rework all socket logic

This is a pretty huge change that will be important for being able to
handle nested or mutually recursive `dispatch()` and `audioMaster()`
calls. This sadly all had to be done in a single commit, so here's a
summary:

- `src/common/sockets.h:Sockets` contains all sockets on both the plugin
  and the Wine host side, and is used to both listen on and connect to
  the sockets.
- Sockets and other temporary files respect `$XDG_RUNTIME_DIR` instead
  of being dumped in `/tmp`.
- All sockets now have a unique endpoint in
  `/run/user/<uid>/yabridge-<plugin_name>-<random_id>/`. This is
  important for when we want to have multiple socket connections for
  handling `dispatch()` and `audioMaster()`.
- Because of the above, we no longer clean up the socket endpoint files
  after the connection gets established during initialization. Instead
  we'll remove the socket base directory when shutting down.

src/plugin/host-process.cpp

@@ -83,7 +83,7 @@ void HostProcess::async_log_pipe_lines(patched_async_pipe& pipe,
 IndividualHost::IndividualHost(boost::asio::io_context& io_context,
                                Logger& logger,
                                fs::path plugin_path,
-                               fs::path socket_endpoint)
+                               const Sockets& sockets)
     : HostProcess(io_context, logger),
       plugin_arch(find_vst_architecture(plugin_path)),
       host_path(find_vst_host(plugin_arch, false)),
@@ -93,7 +93,7 @@ IndividualHost::IndividualHost(boost::asio::io_context& io_context,
 #else
                        plugin_path,
 #endif
-                       socket_endpoint,
+                       sockets.base_dir,
                        bp::env = set_wineprefix(),
                        bp::std_out = stdout_pipe,
                        bp::std_err = stderr_pipe
@@ -127,17 +127,15 @@ void IndividualHost::terminate() {
     host.wait();
 }
 
-GroupHost::GroupHost(
-    boost::asio::io_context& io_context,
-    Logger& logger,
-    fs::path plugin_path,
-    fs::path socket_endpoint,
-    std::string group_name,
-    boost::asio::local::stream_protocol::socket& host_vst_dispatch)
+GroupHost::GroupHost(boost::asio::io_context& io_context,
+                     Logger& logger,
+                     fs::path plugin_path,
+                     Sockets& sockets,
+                     std::string group_name)
     : HostProcess(io_context, logger),
       plugin_arch(find_vst_architecture(plugin_path)),
       host_path(find_vst_host(plugin_arch, true)),
-      host_vst_dispatch(host_vst_dispatch) {
+      sockets(sockets) {
 #ifdef WITH_WINEDBG
     if (plugin_path.string().find(' ') != std::string::npos) {
         logger.log("Warning: winedbg does not support paths containing spaces");
@@ -167,6 +165,7 @@ GroupHost::GroupHost(
         wine_prefix = fs::path(host_env.at("HOME").to_string()) / ".wine";
     }
 
+    const fs::path endpoint_base_dir = sockets.base_dir;
     const fs::path group_socket_path =
         generate_group_endpoint(group_name, wine_prefix, plugin_arch);
     try {
@@ -175,9 +174,10 @@ GroupHost::GroupHost(
         boost::asio::local::stream_protocol::socket group_socket(io_context);
         group_socket.connect(group_socket_path.string());
 
-        write_object(group_socket,
-                     GroupRequest{.plugin_path = plugin_path.string(),
-                                  .socket_path = socket_endpoint.string()});
+        write_object(
+            group_socket,
+            GroupRequest{.plugin_path = plugin_path.string(),
+                         .endpoint_base_dir = endpoint_base_dir.string()});
         const auto response = read_object<GroupResponse>(group_socket);
 
         host_pid = response.pid;
@@ -199,7 +199,7 @@ GroupHost::GroupHost(
         // meantime.
         group_host_connect_handler = std::jthread([&, group_socket_path,
                                                    plugin_path,
-                                                   socket_endpoint]() {
+                                                   endpoint_base_dir]() {
             using namespace std::literals::chrono_literals;
 
             // TODO: Replace this polling with inotify
@@ -214,8 +214,9 @@ GroupHost::GroupHost(
 
                     write_object(
                         group_socket,
-                        GroupRequest{.plugin_path = plugin_path.string(),
-                                     .socket_path = socket_endpoint.string()});
+                        GroupRequest{
+                            .plugin_path = plugin_path.string(),
+                            .endpoint_base_dir = endpoint_base_dir.string()});
                     const auto response =
                         read_object<GroupResponse>(group_socket);
 
@@ -262,7 +263,7 @@ void GroupHost::terminate() {
     // There's no need to manually terminate group host processes as they will
     // shut down automatically after all plugins have exited. Manually closing
     // the dispatch socket will cause the associated plugin to exit.
-    host_vst_dispatch.shutdown(
+    sockets.host_vst_dispatch.shutdown(
         boost::asio::local::stream_protocol::socket::shutdown_both);
-    host_vst_dispatch.close();
+    sockets.host_vst_dispatch.close();
 }

src/plugin/host-process.h

@@ -26,6 +26,7 @@
 #include <thread>
 
 #include "../common/logging.h"
+#include "../common/communication.h"
 #include "utils.h"
 
 /**
@@ -117,7 +118,7 @@ class IndividualHost : public HostProcess {
      *   handled on.
      * @param logger The `Logger` instance the redirected STDIO streams will be
      *   written to.
-     * @param socket_endpoint The endpoint that should be used to communicate
+     * @param sockets The socket endpoints that will be used for communication
      *   with the plugin.
      *
      * @throw std::runtime_error When `plugin_path` does not point to a valid
@@ -126,7 +127,7 @@ class IndividualHost : public HostProcess {
     IndividualHost(boost::asio::io_context& io_context,
                    Logger& logger,
                    boost::filesystem::path plugin_path,
-                   boost::filesystem::path socket_endpoint);
+                   const Sockets& sockets);
 
     PluginArchitecture architecture() override;
     boost::filesystem::path path() override;
@@ -160,19 +161,16 @@ class GroupHost : public HostProcess {
      *   handled on.
      * @param logger The `Logger` instance the redirected STDIO streams will be
      *   written to.
-     * @param socket_endpoint The endpoint that should be used to communicate
-     *   with the plugin.
+     * @param sockets The socket endpoints that will be used for communication
+     *   with the plugin. When the plugin shuts down, we'll terminate the
+     *   dispatch socket contained in this object.
      * @param group_name The name of the plugin group.
-     * @param host_vst_dispatch The socket used to communicate
-     *   `AEffect::dispatcher()` events with this plugin. Will be closed as to
-     *   shut down the plugin.
      */
     GroupHost(boost::asio::io_context& io_context,
               Logger& logger,
               boost::filesystem::path plugin_path,
-              boost::filesystem::path socket_endpoint,
-              std::string group_name,
-              boost::asio::local::stream_protocol::socket& host_vst_dispatch);
+              Sockets& socket_endpoint,
+              std::string group_name);
 
     PluginArchitecture architecture() override;
     boost::filesystem::path path() override;
@@ -191,10 +189,10 @@ class GroupHost : public HostProcess {
     pid_t host_pid;
 
     /**
-     * The associated dispatch socket for the plugin we're hosting. This is used
-     * to terminate the plugin.
+     * The associated sockets for the plugin we're hosting. This is used to
+     * terminate the plugin.
      */
-    boost::asio::local::stream_protocol::socket& host_vst_dispatch;
+    Sockets& sockets;
 
     /**
      * A thread that waits for the group host to have started and then ask it to

src/plugin/plugin-bridge.cpp

@@ -54,32 +54,26 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback)
       // bridge will crash otherwise
       plugin(),
       io_context(),
-      socket_endpoint(generate_plugin_endpoint().string()),
-      socket_acceptor(io_context, socket_endpoint),
-      host_vst_dispatch(io_context),
-      host_vst_dispatch_midi_events(io_context),
-      vst_host_callback(io_context),
-      host_vst_parameters(io_context),
-      host_vst_process_replacing(io_context),
-      host_vst_control(io_context),
+      sockets(io_context,
+              generate_endpoint_base(
+                  vst_plugin_path.filename().replace_extension("").string()),
+              true),
       host_callback_function(host_callback),
       logger(Logger::create_from_environment(
-          create_logger_prefix(socket_endpoint.path()))),
+          create_logger_prefix(sockets.base_dir))),
       wine_version(get_wine_version()),
-      vst_host(
-          config.group
-              ? std::unique_ptr<HostProcess>(
-                    std::make_unique<GroupHost>(io_context,
-                                                logger,
-                                                vst_plugin_path,
-                                                socket_endpoint.path(),
-                                                *config.group,
-                                                host_vst_dispatch))
-              : std::unique_ptr<HostProcess>(
-                    std::make_unique<IndividualHost>(io_context,
+      vst_host(config.group
+                   ? std::unique_ptr<HostProcess>(
+                         std::make_unique<GroupHost>(io_context,
                                                      logger,
                                                      vst_plugin_path,
-                                                     socket_endpoint.path()))),
+                                                     sockets,
+                                                     *config.group))
+                   : std::unique_ptr<HostProcess>(
+                         std::make_unique<IndividualHost>(io_context,
+                                                          logger,
+                                                          vst_plugin_path,
+                                                          sockets))),
       has_realtime_priority(set_realtime_priority()),
       wine_io_handler([&]() { io_context.run(); }) {
     log_init_message();
@@ -107,24 +101,13 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback)
     });
 #endif
 
-    // It's very important that these sockets are connected to in the same
-    // order in the Wine VST host
-    socket_acceptor.accept(host_vst_dispatch);
-    socket_acceptor.accept(host_vst_dispatch_midi_events);
-    socket_acceptor.accept(vst_host_callback);
-    socket_acceptor.accept(host_vst_parameters);
-    socket_acceptor.accept(host_vst_process_replacing);
-    socket_acceptor.accept(host_vst_control);
-
+    // This will block until all sockets have been connected to by the Wine VST
+    // host
+    sockets.connect();
 #ifndef WITH_WINEDBG
     host_guard_handler.request_stop();
 #endif
 
-    // There's no need to keep the socket endpoint file around after accepting
-    // all the sockets, and RAII won't clean these files up for us
-    socket_acceptor.close();
-    fs::remove(socket_endpoint.path());
-
     // Set up all pointers for our `AEffect` struct. We will fill this with data
     // from the VST plugin loaded in Wine at the end of this constructor.
     plugin.ptr3 = this;
@@ -144,8 +127,8 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback)
                 // TODO: Think of a nicer way to structure this and the similar
                 //       handler in `Vst2Bridge::handle_dispatch_midi_events`
                 receive_event(
-                    vst_host_callback, std::pair<Logger&, bool>(logger, false),
-                    [&](Event& event) {
+                    sockets.vst_host_callback,
+                    std::pair<Logger&, bool>(logger, false), [&](Event& event) {
                         // MIDI events sent from the plugin back to the host are
                         // a special case here. They have to sent during the
                         // `processReplacing()` function or else the host will
@@ -181,13 +164,14 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback)
     // call these during its initialization. Any further updates will be sent
     // over the `dispatcher()` socket. This would happen whenever the plugin
     // calls `audioMasterIOChanged()` and after the host calls `effOpen()`.
-    const auto initialization_data = read_object<EventResult>(host_vst_control);
+    const auto initialization_data =
+        read_object<EventResult>(sockets.host_vst_control);
     const auto initialized_plugin =
         std::get<AEffect>(initialization_data.payload);
 
     // After receiving the `AEffect` values we'll want to send the configuration
     // back to complete the startup process
-    write_object(host_vst_control, config);
+    write_object(sockets.host_vst_control, config);
 
     update_aeffect(plugin, initialized_plugin);
 }
@@ -452,10 +436,10 @@ intptr_t PluginBridge::dispatch(AEffect* /*plugin*/,
             intptr_t return_value = 0;
             try {
                 // TODO: Add some kind of timeout?
-                return_value =
-                    send_event(host_vst_dispatch, dispatch_mutex, converter,
-                               std::pair<Logger&, bool>(logger, true), opcode,
-                               index, value, data, option);
+                return_value = send_event(
+                    sockets.host_vst_dispatch, dispatch_mutex, converter,
+                    std::pair<Logger&, bool>(logger, true), opcode, index,
+                    value, data, option);
             } catch (const boost::system::system_error& a) {
                 // Thrown when the socket gets closed because the VST plugin
                 // loaded into the Wine process crashed during shutdown
@@ -478,7 +462,7 @@ intptr_t PluginBridge::dispatch(AEffect* /*plugin*/,
             // thread and socket to pass MIDI events. Otherwise plugins will
             // stop receiving MIDI data when they have an open dropdowns or
             // message box.
-            return send_event(host_vst_dispatch_midi_events,
+            return send_event(sockets.host_vst_dispatch_midi_events,
                               dispatch_midi_events_mutex, converter,
                               std::pair<Logger&, bool>(logger, true), opcode,
                               index, value, data, option);
@@ -538,7 +522,7 @@ intptr_t PluginBridge::dispatch(AEffect* /*plugin*/,
     // and loading plugin state it's much better to have bitsery or our
     // receiving function temporarily allocate a large enough buffer rather than
     // to have a bunch of allocated memory sitting around doing nothing.
-    return send_event(host_vst_dispatch, dispatch_mutex, converter,
+    return send_event(sockets.host_vst_dispatch, dispatch_mutex, converter,
                       std::pair<Logger&, bool>(logger, true), opcode, index,
                       value, data, option);
 }
@@ -555,11 +539,11 @@ void PluginBridge::do_process(T** inputs, T** outputs, int sample_frames) {
     }
 
     const AudioBuffers request{input_buffers, sample_frames};
-    write_object(host_vst_process_replacing, request, process_buffer);
+    write_object(sockets.host_vst_process_replacing, request, process_buffer);
 
     // Write the results back to the `outputs` arrays
-    const auto response =
-        read_object<AudioBuffers>(host_vst_process_replacing, process_buffer);
+    const auto response = read_object<AudioBuffers>(
+        sockets.host_vst_process_replacing, process_buffer);
     const auto& response_buffers =
         std::get<std::vector<std::vector<T>>>(response.buffers);
 
@@ -608,8 +592,8 @@ float PluginBridge::get_parameter(AEffect* /*plugin*/, int index) {
     // called at the same time since  they share the same socket
     {
         std::lock_guard lock(parameters_mutex);
-        write_object(host_vst_parameters, request);
-        response = read_object<ParameterResult>(host_vst_parameters);
+        write_object(sockets.host_vst_parameters, request);
+        response = read_object<ParameterResult>(sockets.host_vst_parameters);
     }
 
     logger.log_get_parameter_response(*response.value);
@@ -625,9 +609,9 @@ void PluginBridge::set_parameter(AEffect* /*plugin*/, int index, float value) {
 
     {
         std::lock_guard lock(parameters_mutex);
-        write_object(host_vst_parameters, request);
+        write_object(sockets.host_vst_parameters, request);
 
-        response = read_object<ParameterResult>(host_vst_parameters);
+        response = read_object<ParameterResult>(sockets.host_vst_parameters);
     }
 
     logger.log_set_parameter_response();
@@ -647,7 +631,8 @@ void PluginBridge::log_init_message() {
              << std::endl;
     init_msg << "realtime:     '" << (has_realtime_priority ? "yes" : "no")
              << "'" << std::endl;
-    init_msg << "socket:       '" << socket_endpoint.path() << "'" << std::endl;
+    init_msg << "sockets:      '" << sockets.base_dir.string() << "'"
+             << std::endl;
     init_msg << "wine prefix:  '";
 
     // If the Wine prefix is manually overridden, then this should be made

src/plugin/plugin-bridge.h

@@ -25,6 +25,7 @@
 
 #include "../common/configuration.h"
 #include "../common/logging.h"
+#include "../common/communication.h"
 #include "host-process.h"
 
 /**
@@ -123,45 +124,7 @@ class PluginBridge {
     void log_init_message();
 
     boost::asio::io_context io_context;
-    boost::asio::local::stream_protocol::endpoint socket_endpoint;
-    boost::asio::local::stream_protocol::acceptor socket_acceptor;
-
-    // The naming convention for these sockets is `<from>_<to>_<event>`. For
-    // instance the socket named `host_vst_dispatch` forwards
-    // `AEffect.dispatch()` calls from the native VST host to the Windows VST
-    // plugin (through the Wine VST host).
-
-    /**
-     * The socket that forwards all `dispatcher()` calls from the VST host to
-     * the plugin.
-     */
-    boost::asio::local::stream_protocol::socket host_vst_dispatch;
-    /**
-     * Used specifically for the `effProcessEvents` opcode. This is needed
-     * because the Win32 API is designed to block during certain GUI
-     * interactions such as resizing a window or opening a dropdown. Without
-     * this MIDI input would just stop working at times.
-     */
-    boost::asio::local::stream_protocol::socket host_vst_dispatch_midi_events;
-    /**
-     * The socket that forwards all `audioMaster()` calls from the Windows VST
-     * plugin to the host.
-     */
-    boost::asio::local::stream_protocol::socket vst_host_callback;
-    /**
-     * Used for both `getParameter` and `setParameter` since they mostly
-     * overlap.
-     */
-    boost::asio::local::stream_protocol::socket host_vst_parameters;
-    boost::asio::local::stream_protocol::socket host_vst_process_replacing;
-
-    /**
-     * A control socket that sends data that is not suitable for the other
-     * sockets. At the moment this is only used to, on startup, send the Windows
-     * VST plugin's `AEffect` object to the native VST plugin, and to then send
-     * the configuration (from `config`) back to the Wine host.
-     */
-    boost::asio::local::stream_protocol::socket host_vst_control;
+    Sockets sockets;
 
     /**
      * The thread that handles host callbacks.

src/plugin/utils.cpp

@@ -22,37 +22,27 @@
 #include <boost/process/search_path.hpp>
 #include <boost/process/system.hpp>
 #include <fstream>
-#include <random>
 #include <sstream>
 
 // Generated inside of the build directory
 #include <src/common/config/config.h>
 
 #include "../common/configuration.h"
+#include "../common/utils.h"
 
 namespace bp = boost::process;
 namespace fs = boost::filesystem;
 
-/**
- * Used for generating random identifiers.
- */
-constexpr char alphanumeric_characters[] =
-    "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
-
-std::string create_logger_prefix(const fs::path& socket_path) {
-    // Use the socket filename as the logger prefix, but strip the `yabridge-`
-    // part since that's redundant
-    std::string socket_name =
-        socket_path.filename().replace_extension().string();
+std::string create_logger_prefix(const fs::path& endpoint_base_dir) {
+    // Use the name of the base directory used for our sockets as the logger
+    // prefix, but strip the `yabridge-` part since that's redundant
+    std::string endpoint_name = endpoint_base_dir.filename().string();
 
     constexpr std::string_view socket_prefix("yabridge-");
-    assert(socket_name.starts_with(socket_prefix));
-    socket_name = socket_name.substr(socket_prefix.size());
+    assert(endpoint_name.starts_with(socket_prefix));
+    endpoint_name = endpoint_name.substr(socket_prefix.size());
 
-    std::ostringstream prefix;
-    prefix << "[" << socket_name << "] ";
-
-    return prefix.str();
+    return "[" + endpoint_name + "] ";
 }
 
 std::optional<fs::path> find_wineprefix() {
@@ -183,39 +173,7 @@ boost::filesystem::path generate_group_endpoint(
     }
     socket_name << ".sock";
 
-    return fs::temp_directory_path() / socket_name.str();
-}
-
-fs::path generate_plugin_endpoint() {
-    const auto plugin_name =
-        find_vst_plugin().filename().replace_extension("").string();
-
-    std::random_device random_device;
-    std::mt19937 rng(random_device());
-    fs::path candidate_endpoint;
-    do {
-        std::string random_id;
-        std::sample(
-            alphanumeric_characters,
-            alphanumeric_characters + strlen(alphanumeric_characters) - 1,
-            std::back_inserter(random_id), 8, rng);
-
-        // We'll get rid of the file descriptors immediately after accepting the
-        // sockets, so putting them inside of a subdirectory would only leave
-        // behind an empty directory
-        std::ostringstream socket_name;
-        socket_name << "yabridge-" << plugin_name << "-" << random_id
-                    << ".sock";
-
-        candidate_endpoint = fs::temp_directory_path() / socket_name.str();
-    } while (fs::exists(candidate_endpoint));
-
-    // TODO: Should probably try creating the endpoint right here and catch any
-    //       exceptions since this could technically result in a race condition
-    //       when two instances of yabridge decide to use the same endpoint name
-    //       at the same time
-
-    return candidate_endpoint;
+    return get_temporary_directory() / socket_name.str();
 }
 
 fs::path get_this_file_location() {
@@ -243,7 +201,7 @@ std::string get_wine_version() {
 
     bp::environment env = boost::this_process::environment();
     if (!env["WINELOADER"].empty()) {
-        wine_command = env.get("WINELOADER");
+        wine_command = env["WINELOADER"].to_string();
     }
 
     bp::ipstream output;
@@ -271,13 +229,13 @@ std::string get_wine_version() {
 
 std::string join_quoted_strings(std::vector<std::string>& strings) {
     bool is_first = true;
-    std::ostringstream joined_strigns{};
+    std::ostringstream joined_strings{};
     for (const auto& option : strings) {
-        joined_strigns << (is_first ? "'" : ", '") << option << "'";
+        joined_strings << (is_first ? "'" : ", '") << option << "'";
         is_first = false;
     }
 
-    return joined_strigns.str();
+    return joined_strings.str();
 }
 
 Configuration load_config_for(const fs::path& yabridge_path) {

src/plugin/utils.h

@@ -46,15 +46,17 @@ class patched_async_pipe : public boost::process::async_pipe {
 enum class PluginArchitecture { vst_32, vst_64 };
 
 /**
- * Create a logger prefix based on the unique socket path for easy
- * identification. The socket path contains both the plugin's name and a unique
- * identifier.
+ * Create a logger prefix based on the endpoint base directory used for the
+ * sockets for easy identification. This will result in a prefix of the form
+ * `[<plugin_name>-<random_id>] `.
  *
- * @param socket_path The path to the socket endpoint in use.
+ * @param endpoint_base_dir A directory name generated by
+ *   `generate_endpoint_base()`.
  *
  * @return A prefix string for log messages.
  */
-std::string create_logger_prefix(const boost::filesystem::path& socket_path);
+std::string create_logger_prefix(
+    const boost::filesystem::path& endpoint_base_dir);
 
 /**
  * Determine the architecture of a VST plugin (or rather, a .dll file) based on
@@ -117,10 +119,11 @@ std::optional<boost::filesystem::path> find_wineprefix();
 /**
  * Generate the group socket endpoint name used based on the name of the group,
  * the Wine prefix in use and the plugin architecture. The resulting format is
- * `/tmp/yabridge-group-<group_name>-<wine_prefix_id>-<architecture>.sock`. In
- * this socket name the `wine_prefix_id` is a numerical hash based on the Wine
- * prefix in use. This way the same group name can be used for multiple Wine
- * prefixes and for both 32 and 64 bit plugins without clashes.
+ * in the form
+ * `/run/user/<uid>/yabridge-group-<group_name>-<wine_prefix_id>-<architecture>.sock`.
+ * In this socket name the `wine_prefix_id` is a numerical hash based on the
+ * Wine prefix in use. This way the same group name can be used for multiple
+ * Wine prefixes and for both 32 and 64 bit plugins without clashes.
  *
  * @param group_name The name of the plugin group.
  * @param wine_prefix The name of the Wine prefix in use. This should be
@@ -140,16 +143,6 @@ boost::filesystem::path generate_group_endpoint(
     const boost::filesystem::path& wine_prefix,
     const PluginArchitecture architecture);
 
-/**
- * Generate a unique name for the Unix domain socket endpoint based on the VST
- * plugin's name. This will also generate the parent directory if it does not
- * yet exist since we're using this in the constructor's initializer list.
- *
- * @return A path to a not yet existing Unix domain socket endpoint.
- * @throw std::runtime_error If no matching .dll file could be found.
- */
-boost::filesystem::path generate_plugin_endpoint();
-
 /**
  * Return a path to this `.so` file. This can be used to find out from where
  * this link to or copy of `libyabridge.so` was loaded.