Use std::jthread

src/plugin/host-process.cpp

@@ -187,9 +187,9 @@ GroupHost::GroupHost(
         // process to start depending on Wine's current state. We'll defer
         // this to a thread so we can finish the rest of the startup in the
         // meantime.
-        group_host_connect_handler = std::thread([&, group_socket_path,
-                                                  plugin_path,
-                                                  socket_endpoint]() {
+        group_host_connect_handler = std::jthread([&, group_socket_path,
+                                                   plugin_path,
+                                                   socket_endpoint]() {
             using namespace std::literals::chrono_literals;
 
             // TODO: Replace this polling with inotify
@@ -221,14 +221,6 @@ GroupHost::GroupHost(
     }
 }
 
-GroupHost::~GroupHost() {
-    // This thread was briefly used to issue the host request if we had to start
-    // a new group host process
-    if (group_host_connect_handler.joinable()) {
-        group_host_connect_handler.join();
-    }
-}
-
 PluginArchitecture GroupHost::architecture() {
     return plugin_arch;
 }

src/plugin/host-process.h

@@ -171,8 +171,6 @@ class GroupHost : public HostProcess {
               std::string group_name,
               boost::asio::local::stream_protocol::socket& host_vst_dispatch);
 
-    ~GroupHost();
-
     PluginArchitecture architecture() override;
     boost::filesystem::path path() override;
     bool running() override;
@@ -204,5 +202,5 @@ class GroupHost : public HostProcess {
      * TODO: Replace the polling with inotify to prevent delays and to reduce
      *       wasting resources
      */
-    std::thread group_host_connect_handler;
+    std::jthread group_host_connect_handler;
 };

src/plugin/plugin-bridge.cpp

@@ -86,13 +86,10 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback)
     // when it is not. The alternative would be to rewrite this to using
     // `async_accept`, Boost.Asio timers, and another IO context, but I feel
     // like this a much simpler solution.
-    std::thread([&]() {
+    std::jthread host_guard_handler([&](std::stop_token st) {
         using namespace std::literals::chrono_literals;
 
-        while (true) {
-            if (finished_accepting_sockets) {
-                return;
-            }
+        while (!st.stop_requested()) {
             if (!vst_host->running()) {
                 logger.log(
                     "The Wine host process has exited unexpectedly. Check the "
@@ -102,7 +99,7 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback)
 
             std::this_thread::sleep_for(1s);
         }
-    }).detach();
+    });
 #endif
 
     // It's very important that these sockets are connected to in the same
@@ -112,7 +109,11 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback)
     socket_acceptor.accept(vst_host_callback);
     socket_acceptor.accept(host_vst_parameters);
     socket_acceptor.accept(host_vst_process_replacing);
-    finished_accepting_sockets = true;
+
+#ifndef WITH_WINEDBG
+    host_guard_handler.request_stop();
+    host_guard_handler.detach();
+#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
@@ -131,7 +132,7 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback)
     // For our communication we use simple threads and blocking operations
     // instead of asynchronous IO since communication has to be handled in
     // lockstep anyway
-    host_callback_handler = std::thread([&]() {
+    host_callback_handler = std::jthread([&]() {
         while (true) {
             try {
                 // TODO: Think of a nicer way to structure this and the similar
@@ -456,11 +457,6 @@ intptr_t PluginBridge::dispatch(AEffect* /*plugin*/,
             // been caused by pipes and sockets being closed.
             io_context.stop();
 
-            // These threads should now be finished because we've forcefully
-            // terminated the Wine process, interupting their socket operations
-            host_callback_handler.join();
-            wine_io_handler.join();
-
             delete this;
 
             return return_value;

src/plugin/plugin-bridge.h

@@ -128,19 +128,10 @@ class PluginBridge {
     boost::asio::local::stream_protocol::socket host_vst_parameters;
     boost::asio::local::stream_protocol::socket host_vst_process_replacing;
 
-    /**
-     * Whether we're done accepting sockets. The plugin may hang indefinitely if
-     * the Wine process fails to start, since then nothing will connect to our
-     * sockets. While we're waiting for our sockets we'll periodically poll the
-     * Wine process to see if it's still running, and terminate the socket
-     * accepting if it is not.
-     */
-    std::atomic_bool finished_accepting_sockets;
-
     /**
      * The thread that handles host callbacks.
      */
-    std::thread host_callback_handler;
+    std::jthread host_callback_handler;
 
     /**
      * A binary semaphore to prevent race conditions from the dispatch function
@@ -185,7 +176,7 @@ class PluginBridge {
      * Runs the Boost.Asio `io_context` thread for logging the Wine process
      * STDOUT and STDERR messages.
      */
-    std::thread wine_io_handler;
+    std::jthread wine_io_handler;
 
     /**
      * A scratch buffer for sending and receiving data during `process` and

src/wine-host/bridges/group.cpp

@@ -102,12 +102,11 @@ GroupBridge::GroupBridge(boost::filesystem::path group_socket_path)
     async_log_pipe_lines(stdout_redirect.pipe, stdout_buffer, "[STDOUT] ");
     async_log_pipe_lines(stderr_redirect.pipe, stderr_buffer, "[STDERR] ");
 
-    stdio_handler = std::thread([&]() { stdio_context.run(); });
+    stdio_handler = std::jthread([&]() { stdio_context.run(); });
 }
 
 GroupBridge::~GroupBridge() {
     stdio_context.stop();
-    stdio_handler.join();
 }
 
 void GroupBridge::handle_plugin_dispatch(const GroupRequest request) {
@@ -129,8 +128,7 @@ void GroupBridge::handle_plugin_dispatch(const GroupRequest request) {
     boost::asio::post(plugin_context, [&, request]() {
         std::lock_guard lock(active_plugins_mutex);
 
-        auto& [thread, bridge] = active_plugins.at(request);
-        thread.join();
+        // The join is implicit because we're using std::jthread
         active_plugins.erase(request);
     });
 
@@ -220,7 +218,7 @@ void GroupBridge::accept_requests() {
                 // socket on another thread. The actual event handling will
                 // still occur within this IO context.
                 active_plugins[request] =
-                    std::pair(std::thread([&, request]() {
+                    std::pair(std::jthread([&, request]() {
                                   handle_plugin_dispatch(request);
                               }),
                               std::move(bridge));

src/wine-host/bridges/group.h

@@ -240,7 +240,7 @@ class GroupBridge {
     /**
      * A thread that runs the `stdio_context` loop.
      */
-    std::thread stdio_handler;
+    std::jthread stdio_handler;
 
     boost::asio::local::stream_protocol::endpoint group_socket_endpoint;
     /**
@@ -255,14 +255,9 @@ class GroupBridge {
      * 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.
-     *
-     * 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,
-                       std::pair<std::thread, std::unique_ptr<Vst2Bridge>>>
+                       std::pair<std::jthread, std::unique_ptr<Vst2Bridge>>>
         active_plugins;
     /**
      * A mutex to prevent two threads from simultaneously accessing the plugins

src/wine-host/individual-host.cpp

@@ -94,7 +94,7 @@ int __cdecl main(int argc, char* argv[]) {
 
     // We'll listen for `dispatcher()` calls on a different thread, but the
     // actual events will still be executed within the IO context
-    std::thread dispatch_handler([&]() { bridge->handle_dispatch(); });
+    std::jthread dispatch_handler([&]() { bridge->handle_dispatch(); });
 
     // Handle Win32 messages and X11 events on a timer, just like in
     // `GroupBridge::async_handle_events()``
@@ -102,7 +102,6 @@ int __cdecl main(int argc, char* argv[]) {
     async_handle_events(events_timer, *bridge);
 
     io_context.run();
-    dispatch_handler.join();
 }
 
 void async_handle_events(boost::asio::steady_timer& timer, Vst2Bridge& bridge) {

src/wine-host/utils.h

@@ -45,8 +45,8 @@
  *   converting stateless lambdas to this format, but clang (as used for IDE
  *   tooling) does not.
  *
- * @note This should be used instead of `std::thread` whenever the thread
- *   directly calls third party library code, i.e. `LoadLibrary()`,
+ * @note This should be used instead of `std::thread` or `std::jthread` whenever
+ *   the thread directly calls third party library code, i.e. `LoadLibrary()`,
  *   `FreeLibrary()`, the plugin's entry point, or any of the `AEffect::*()`
  *   functions.
  */