From b036230067eafc65a1b64cd5e5f9769247548957 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Sat, 27 Mar 2021 17:41:15 +0100 Subject: [PATCH] Work around a regression in Wine 6.5 Killing a Wine process no longer terminates its threads, see the changelog entry and NOTE for more information. --- CHANGELOG.md | 9 ++++++++- src/plugin/bridges/common.h | 11 ++++++----- src/plugin/host-process.cpp | 26 +++++++++++++++++++------- src/plugin/host-process.h | 31 ++++++++++++++++++++----------- 4 files changed, 53 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8db6c7d..74d29560 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,14 @@ Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] -### Yabridgectl +### Fixed + +- Work around a regression in Wine 6.5 that would prevent yabridge from shutting + down. With Wine 6.5 terminating a Wine process no longer terminates its + threads, which would cause yabridge's plugin and host components to wait for + each other to shut down. + +### yabridgectl - Minor spelling fixes. diff --git a/src/plugin/bridges/common.h b/src/plugin/bridges/common.h index b4d84501..a8dcce6b 100644 --- a/src/plugin/bridges/common.h +++ b/src/plugin/bridges/common.h @@ -84,11 +84,12 @@ class PluginBridge { io_context, generic_logger, info, - HostRequest{.plugin_type = plugin_type, - .plugin_path = - info.windows_plugin_path.string(), - .endpoint_base_dir = - sockets.base_dir.string()}))), + HostRequest{ + .plugin_type = plugin_type, + .plugin_path = + info.windows_plugin_path.string(), + .endpoint_base_dir = sockets.base_dir.string()}, + sockets))), has_realtime_priority(has_realtime_priority_promise.get_future()), wine_io_handler([&]() { // We no longer run this thread with realtime scheduling because diff --git a/src/plugin/host-process.cpp b/src/plugin/host-process.cpp index 621ac182..96e5982e 100644 --- a/src/plugin/host-process.cpp +++ b/src/plugin/host-process.cpp @@ -56,8 +56,13 @@ bp::child launch_host(fs::path host_path, Args&&... args) { bp::posix::use_vfork, std::forward(args)...); } -HostProcess::HostProcess(boost::asio::io_context& io_context, Logger& logger) - : stdout_pipe(io_context), stderr_pipe(io_context), logger(logger) { +HostProcess::HostProcess(boost::asio::io_context& io_context, + Logger& logger, + Sockets& sockets) + : stdout_pipe(io_context), + stderr_pipe(io_context), + sockets(sockets), + logger(logger) { // Print the Wine host's STDOUT and STDERR streams to the log file. This // should be done before trying to accept the sockets as otherwise we will // miss all output. @@ -68,8 +73,9 @@ HostProcess::HostProcess(boost::asio::io_context& io_context, Logger& logger) IndividualHost::IndividualHost(boost::asio::io_context& io_context, Logger& logger, const PluginInfo& plugin_info, - const HostRequest& host_request) - : HostProcess(io_context, logger), + const HostRequest& host_request, + Sockets& sockets) + : HostProcess(io_context, logger, sockets), plugin_info(plugin_info), host_path(find_vst_host(plugin_info.native_library_path, plugin_info.plugin_arch, @@ -109,6 +115,13 @@ bool IndividualHost::running() { } void IndividualHost::terminate() { + // NOTE: This technically shouldn't be needed, but in Wine 6.5 sending + // SIGKILL to a Wine process no longer terminates the threads spawned + // by that process, so if we don't manually close the sockets there + // will still be threads listening on those sockets which in turn also + // prevents us from joining our `std::jthread`s on the plugin side. + sockets.close(); + host.terminate(); // NOTE: This leaves a zombie, because Boost.Process will actually not call // `wait()` after we have terminated the process. @@ -121,12 +134,11 @@ GroupHost::GroupHost(boost::asio::io_context& io_context, const HostRequest& host_request, Sockets& sockets, std::string group_name) - : HostProcess(io_context, logger), + : HostProcess(io_context, logger, sockets), plugin_info(plugin_info), host_path(find_vst_host(plugin_info.native_library_path, plugin_info.plugin_arch, - true)), - sockets(sockets) { + true)) { #ifdef WITH_WINEDBG if (plugin_info.windows_plugin_path.string().find(' ') != std::string::npos) { diff --git a/src/plugin/host-process.h b/src/plugin/host-process.h index 1868be9f..2e8e669c 100644 --- a/src/plugin/host-process.h +++ b/src/plugin/host-process.h @@ -68,8 +68,13 @@ class HostProcess { * handled on. * @param logger The `Logger` instance the redirected STDIO streams will be * written to. + * @param sockets The socket endpoints that will be used for communication + * with the plugin. When the plugin shuts down, we'll close all of the + * sockets used by the plugin. */ - HostProcess(boost::asio::io_context& io_context, Logger& logger); + HostProcess(boost::asio::io_context& io_context, + Logger& logger, + Sockets& sockets); /** * The STDOUT stream of the Wine process we can forward to the logger. @@ -80,6 +85,12 @@ class HostProcess { */ patched_async_pipe stderr_pipe; + /** + * The associated sockets for the plugin we're hosting. This is used to + * terminate the plugin. + */ + Sockets& sockets; + private: /** * The logger the Wine output will be written to. @@ -108,6 +119,9 @@ class IndividualHost : public HostProcess { * @param host_request The information about the plugin we should launch a * host process for. The values in the struct will be used as command line * arguments. + * @param sockets The socket endpoints that will be used for communication + * with the plugin. When the plugin shuts down, we'll close all of the + * sockets used by the plugin. * * @throw std::runtime_error When `plugin_path` does not point to a valid * 32-bit or 64-bit .dll file. @@ -115,7 +129,8 @@ class IndividualHost : public HostProcess { IndividualHost(boost::asio::io_context& io_context, Logger& logger, const PluginInfo& plugin_info, - const HostRequest& host_request); + const HostRequest& host_request, + Sockets& sockets); boost::filesystem::path path() override; bool running() override; @@ -153,15 +168,15 @@ class GroupHost : public HostProcess { * @param host_request The information about the plugin we should launch a * host process for. This object will be sent to the group host process. * @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. + * with the plugin. When the plugin shuts down, we'll close all of the + * sockets used by the plugin. * @param group_name The name of the plugin group. */ GroupHost(boost::asio::io_context& io_context, Logger& logger, const PluginInfo& plugin_info, const HostRequest& host_request, - Sockets& socket_endpoint, + Sockets& sockets, std::string group_name); boost::filesystem::path path() override; @@ -190,12 +205,6 @@ class GroupHost : public HostProcess { */ std::atomic_bool startup_failed; - /** - * The associated sockets for the plugin we're hosting. This is used to - * terminate the plugin. - */ - Sockets& sockets; - /** * A thread that waits for the group host to have started and then ask it to * host our plugin. This is used to defer the request since it may take a