From 4cc44c3cf716d1bcc78a0de3720aac9e2c2ef653 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Wed, 20 Jan 2021 12:42:59 +0100 Subject: [PATCH] Make the mutual recursion mechanism safer By directly stopping the IO context there was a chance that a task would get cancelled outright if all stars aligned in the wrong way. Stopping the IO context could happen between posting the task to the context and waiting for it. This approach is much safer as we cannot drop any work this way. --- src/plugin/bridges/vst3-impls/plug-view-proxy.h | 12 +++++++++--- src/wine-host/bridges/vst3.h | 12 +++++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/plugin/bridges/vst3-impls/plug-view-proxy.h b/src/plugin/bridges/vst3-impls/plug-view-proxy.h index 05131b0d..772e5631 100644 --- a/src/plugin/bridges/vst3-impls/plug-view-proxy.h +++ b/src/plugin/bridges/vst3-impls/plug-view-proxy.h @@ -235,6 +235,11 @@ class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy { mutual_recursion_contexts.push_back(current_io_context); } + // Instead of directly stopping the IO context, we'll reset this work + // guard instead. This prevents us from accidentally cancelling any + // outstanding tasks. + auto work_guard = boost::asio::make_work_guard(*current_io_context); + // We will call the function from another thread so we can handle calls // to from this thread std::promise response_promise{}; @@ -242,9 +247,11 @@ class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy { const TResponse response = bridge.send_message(object); // Stop accepting additional work to be run from the calling thread - // once we receive a response + // once we receive a response. By resetting the work guard we do not + // cancel any pending tasks, but `current_io_context->run()` will + // stop blocking eventually. std::lock_guard lock(mutual_recursion_contexts_mutex); - current_io_context->stop(); + work_guard.reset(); mutual_recursion_contexts.erase( std::find(mutual_recursion_contexts.begin(), mutual_recursion_contexts.end(), current_io_context)); @@ -254,7 +261,6 @@ class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy { // Accept work from the other thread until we receive a response, at // which point the context will be stopped - auto work_guard = boost::asio::make_work_guard(*current_io_context); current_io_context->run(); return response_promise.get_future().get(); diff --git a/src/wine-host/bridges/vst3.h b/src/wine-host/bridges/vst3.h index f84b6726..c5ab1c25 100644 --- a/src/wine-host/bridges/vst3.h +++ b/src/wine-host/bridges/vst3.h @@ -263,6 +263,11 @@ class Vst3Bridge : public HostBridge { mutual_recursion_contexts.push_back(current_io_context); } + // Instead of directly stopping the IO context, we'll reset this work + // guard instead. This prevents us from accidentally cancelling any + // outstanding tasks. + auto work_guard = boost::asio::make_work_guard(*current_io_context); + // We will call the function from another thread so we can handle calls // to from this thread std::promise response_promise{}; @@ -270,9 +275,11 @@ class Vst3Bridge : public HostBridge { const TResponse response = send_message(object); // Stop accepting additional work to be run from the calling thread - // once we receive a response + // once we receive a response. By resetting the work guard we do not + // cancel any pending tasks, but `current_io_context->run()` will + // stop blocking eventually. std::lock_guard lock(mutual_recursion_contexts_mutex); - current_io_context->stop(); + work_guard.reset(); mutual_recursion_contexts.erase( std::find(mutual_recursion_contexts.begin(), mutual_recursion_contexts.end(), current_io_context)); @@ -282,7 +289,6 @@ class Vst3Bridge : public HostBridge { // Accept work from the other thread until we receive a response, at // which point the context will be stopped - auto work_guard = boost::asio::make_work_guard(*current_io_context); current_io_context->run(); return response_promise.get_future().get();