From 32b3e106b18c60020e7a2a30b4c0fcb2df687813 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Mon, 28 Sep 2020 22:29:10 +0200 Subject: [PATCH] Fixed potential use-after-free on detached threads This could sometimes cause REAPER's plugin scanning to crash when the stars aligned in the wrong way since the stop token would no longer exist. --- CHANGELOG.md | 3 +++ src/plugin/plugin-bridge.cpp | 5 ++--- src/plugin/plugin-bridge.h | 9 +++++++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 475e756e..de3684e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ Versioning](https://semver.org/spec/v2.0.0.html). ### Fixed +- Fixed a potential crash that could happen if the host would unload a plugin + immediately after its initialization. This issue affected the plugin scanning + in _REAPER_. - Fixed parsing order of `yabridge.toml`. Sections were not always read from top to bottom like they should be, which could cause incorrect setting overrides. - Fixed an initialization error when using plugin groups for plugins that are diff --git a/src/plugin/plugin-bridge.cpp b/src/plugin/plugin-bridge.cpp index 13e032a5..88977bbb 100644 --- a/src/plugin/plugin-bridge.cpp +++ b/src/plugin/plugin-bridge.cpp @@ -91,7 +91,7 @@ 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::jthread host_guard_handler([&](std::stop_token st) { + host_guard_handler = std::jthread([&](std::stop_token st) { using namespace std::literals::chrono_literals; while (!st.stop_requested()) { @@ -102,7 +102,7 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback) std::terminate(); } - std::this_thread::sleep_for(1s); + std::this_thread::sleep_for(20ms); } }); #endif @@ -118,7 +118,6 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback) #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 diff --git a/src/plugin/plugin-bridge.h b/src/plugin/plugin-bridge.h index fbac9cc2..6b3fc57e 100644 --- a/src/plugin/plugin-bridge.h +++ b/src/plugin/plugin-bridge.h @@ -208,6 +208,15 @@ class PluginBridge { */ std::unique_ptr vst_host; + /** + * A thread used during the initialisation process to terminate listening on + * the sockets if the Wine process cannot start for whatever reason. This + * has to be defined here instead of in the constructor we can't simply + * detach the thread as it has to check whether the VST host is still + * running. + */ + std::jthread host_guard_handler; + /** * Whether this process runs with realtime priority. We'll set this _after_ * spawning the Wine process because from my testing running wineserver with