diff --git a/src/plugin/host-process.cpp b/src/plugin/host-process.cpp index 94988f4f..f7c1c33e 100644 --- a/src/plugin/host-process.cpp +++ b/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; } diff --git a/src/plugin/host-process.h b/src/plugin/host-process.h index 04a542fe..859a8af4 100644 --- a/src/plugin/host-process.h +++ b/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; }; diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index 59f6df3a..514ddc6f 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/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; diff --git a/src/plugin/plugin-bridge.h b/src/plugin/plugin-bridge.h index ccc51cc1..b271e95d 100644 --- a/src/plugin/plugin-bridge.h +++ b/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 diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index 503e8682..b464c3d8 100644 --- a/src/wine-host/bridges/group.cpp +++ b/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)); diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h index 7c0b55b9..9768daef 100644 --- a/src/wine-host/bridges/group.h +++ b/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>> + std::pair>> active_plugins; /** * A mutex to prevent two threads from simultaneously accessing the plugins diff --git a/src/wine-host/individual-host.cpp b/src/wine-host/individual-host.cpp index 9b2bcc8f..592f411b 100644 --- a/src/wine-host/individual-host.cpp +++ b/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) { diff --git a/src/wine-host/utils.h b/src/wine-host/utils.h index 5da96c85..14826e85 100644 --- a/src/wine-host/utils.h +++ b/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. */