Revert "Separate mutual recursion on GUI and other threads"

This reverts commit a495f1a67f.

This ended up not being an issue. What we _do_ have to do, sadly, is to
have a mutual recursion context stack per plugin. Otherwise multiple
plugin instances can deadlock eachother.
This commit is contained in:
Robbert van der Helm
2021-04-29 13:52:37 +02:00
parent a2e3e691d2
commit 45d83ad9a1
5 changed files with 70 additions and 106 deletions
@@ -61,7 +61,7 @@ Vst3ComponentHandlerProxyImpl::endEdit(Steinberg::Vst::ParamID id) {
tresult PLUGIN_API
Vst3ComponentHandlerProxyImpl::restartComponent(int32 flags) {
return bridge.send_mutually_recursive_message<false>(
return bridge.send_mutually_recursive_message(
YaComponentHandler::RestartComponent{
.owner_instance_id = owner_instance_id(), .flags = flags});
}
@@ -56,7 +56,7 @@ Vst3ConnectionPointProxyImpl::notify(Steinberg::Vst::IMessage* message) {
// need to use our mutual recursion mechanism. Luckily only Ardour uses
// connection proxies, so if this ends up breaking something it will
// only affect Ardour.
return bridge.send_mutually_recursive_message<true>(
return bridge.send_mutually_recursive_message(
YaConnectionPoint::Notify{.instance_id = owner_instance_id(),
.message_ptr = YaMessagePtr(*message)});
} else {
@@ -46,9 +46,8 @@ Vst3PlugFrameProxyImpl::resizeView(Steinberg::IPlugView* /*view*/,
// We have to use this special sending function here so we can handle
// calls to `IPlugView::onSize()` from this same thread (the UI thread).
// See the docstring for more information.
return bridge.send_mutually_recursive_message<true>(
YaPlugFrame::ResizeView{.owner_instance_id = owner_instance_id(),
.new_size = *newSize});
return bridge.send_mutually_recursive_message(YaPlugFrame::ResizeView{
.owner_instance_id = owner_instance_id(), .new_size = *newSize});
} else {
std::cerr
<< "WARNING: Null pointer passed to 'IPlugFrame::resizeView()'"
+36 -28
View File
@@ -355,11 +355,12 @@ void Vst3Bridge::run() {
// much slower in Ardour, but there's no other non-hacky
// solution for this (and bypassing Ardour's connection
// proxies sort of goes against the idea behind yabridge)
return do_mutual_recursion_on_gui_thread<tresult>([&]() {
return object_instances[request.instance_id]
.connection_point->notify(
request.message_ptr.get_original());
});
return do_mutual_recursion_or_handle_in_main_context<tresult>(
[&]() {
return object_instances[request.instance_id]
.connection_point->notify(
request.message_ptr.get_original());
});
},
[&](YaContextMenuTarget::ExecuteMenuItem& request)
-> YaContextMenuTarget::ExecuteMenuItem::Response {
@@ -780,10 +781,11 @@ void Vst3Bridge::run() {
// not run from the GUI thread
Steinberg::ViewRect size{};
const tresult result =
do_mutual_recursion_on_gui_thread<tresult>([&]() {
return object_instances[request.owner_instance_id]
.plug_view_instance->plug_view->getSize(&size);
});
do_mutual_recursion_or_handle_in_main_context<tresult>(
[&]() {
return object_instances[request.owner_instance_id]
.plug_view_instance->plug_view->getSize(&size);
});
return YaPlugView::GetSizeResponse{.result = result,
.size = std::move(size)};
@@ -798,11 +800,12 @@ void Vst3Bridge::run() {
// code on the same thread that's currently waiting for a
// response to the message it sent. See the docstring of
// this function for more information on how this works.
return do_mutual_recursion_on_gui_thread<tresult>([&]() {
return object_instances[request.owner_instance_id]
.plug_view_instance->plug_view->onSize(
&request.new_size);
});
return do_mutual_recursion_or_handle_in_main_context<tresult>(
[&]() {
return object_instances[request.owner_instance_id]
.plug_view_instance->plug_view->onSize(
&request.new_size);
});
},
[&](const YaPlugView::OnFocus& request)
-> YaPlugView::OnFocus::Response {
@@ -845,19 +848,21 @@ void Vst3Bridge::run() {
-> YaPlugView::CanResize::Response {
// To prevent weird behaviour we'll perform all size related
// functions from the GUI thread, including this one
return do_mutual_recursion_on_gui_thread<tresult>([&]() {
return object_instances[request.owner_instance_id]
.plug_view_instance->plug_view->canResize();
});
return do_mutual_recursion_or_handle_in_main_context<tresult>(
[&]() {
return object_instances[request.owner_instance_id]
.plug_view_instance->plug_view->canResize();
});
},
[&](YaPlugView::CheckSizeConstraint& request)
-> YaPlugView::CheckSizeConstraint::Response {
const tresult result =
do_mutual_recursion_on_gui_thread<tresult>([&]() {
return object_instances[request.owner_instance_id]
.plug_view_instance->plug_view->checkSizeConstraint(
&request.rect);
});
do_mutual_recursion_or_handle_in_main_context<tresult>(
[&]() {
return object_instances[request.owner_instance_id]
.plug_view_instance->plug_view
->checkSizeConstraint(&request.rect);
});
return YaPlugView::CheckSizeConstraintResponse{
.result = result, .updated_rect = std::move(request.rect)};
@@ -1303,11 +1308,14 @@ size_t Vst3Bridge::register_object_instance(
// handled from the same thread to prevent
// deadlocks caused by mutually recursive function
// calls.
return do_mutual_recursion_on_off_thread<tresult>(
[&]() {
return object_instances[request.instance_id]
.component->setActive(request.state);
});
// TODO: Check if this causes any issues when activating
// plugins while simultaneously resizing another
// instance of the same plugin
return do_mutual_recursion_or_handle_in_main_context<
tresult>([&]() {
return object_instances[request.instance_id]
.component->setActive(request.state);
});
},
[&](const YaPrefetchableSupport::GetPrefetchableSupport&
request)
+30 -73
View File
@@ -245,19 +245,18 @@ class Vst3Bridge : public HostBridge {
/**
* Spawn a new thread and call `send_message()` from there, and then handle
* functions passed by calls to `do_mutual_recursion_on_gui_thread()` and
* `do_mutual_recursion_on_off_thread()` on this thread until the original
* message we're trying to send has successfully returned. This is a very
* specific solution to a very specific problem. We need the GUI version for
* when a plugin wants to resize itself, it will call
* `IPlugFrame::resizeView()` from within the WIn32 message loop. The host
* will then call `IPlugView::onSize()` on the plugin's `IPlugView` to
* actually resize the plugin. The issue is that that call to
* `IPlugView::onSize()` has to be handled from the UI thread, but in this
* sequence that thread is being blocked by a call to
* functions passed by calls to
* `do_mutual_recursion_or_handle_in_main_context()` on this thread until
* the original message we're trying to send has succeeded. This is a very
* specific solution to a very specific problem. When a plugin wants to
* resize itself, it will call `IPlugFrame::resizeView()` from within the
* WIn32 message loop. The host will then call `IPlugView::onSize()` on the
* plugin's `IPlugView` to actually resize the plugin. The issue is that
* that call to `IPlugView::onSize()` has to be handled from the UI thread,
* but in this sequence that thread is being blocked by a call to
* `IPlugFrame::resizeView()`.
*
* The off-thread non-GUI version is needed for when a plugin calls
* We also need to use this for when a plugin calls
* `IComponentHandler::restartComponent()` to change the latency, and when
* the host then calls `IAudioProcessor::setActive()` in response to that to
* restart the plugin. Otherwise this will lead to an infinite loop.
@@ -267,31 +266,11 @@ class Vst3Bridge : public HostBridge {
* context.
*
* We apply the same trick in `Vst3PlugViewProxyImpl`.
*
* @tparam on_gui_thread Whether we are currently on the GUI thread. This
* determines which IO contexts we'll use. When this mutual recursion
* sequence _has_ to be executed from the GUI thread, pass `true`, and use
* `do_mutual_recursion_on_gui_thread()` to handle incoming requests that
* should be handled from this same thread. Otherwise pass `false` and use
* `do_mutual_recursion_on_off_thread()` to make sure that these messages
* don't end up on the GUI thread.
*/
template <bool on_gui_thread, typename T>
template <typename T>
typename T::Response send_mutually_recursive_message(const T& object) {
using TResponse = typename T::Response;
// Keeping mutually recursive tasks that have to be run on the GUI
// threads and tasks that cna be run on any thread separate will avoid
// any potential clashes, even if I wasn't able to find any problematic
// situations when I tested this with only one stack of IO contexts
std::vector<std::shared_ptr<boost::asio::io_context>>&
mutual_recursion_contexts =
on_gui_thread ? gui_thread_mutual_recursion_contexts
: off_thread_mutual_recursion_contexts;
std::mutex& mutual_recursion_contexts_mutex =
on_gui_thread ? gui_thread_mutual_recursion_contexts_mutex
: off_thread_mutual_recursion_contexts_mutex;
// This IO context will accept incoming calls from `run_gui_task()`
// until we receive a response. We keep these on a stack as we need to
// support multiple levels of mutual recursion. This could happen during
@@ -337,19 +316,16 @@ class Vst3Bridge : public HostBridge {
/**
* Crazy functions ask for crazy naming. This is the other part of
* `send_mutually_recursive_message()`, for executing mutually recursive
* functions on the GUI thread. If another thread is currently calling that
* function (from the UI thread), then we'll execute `f` from the UI thread
* using the IO context started in the above function. Otherwise `f` will be
* run on the UI thread through `main_context` as usual.
*
* You should pass `on_gui_thread = true` to
* `Vst3Bridge::send_mutually_recursive_message` when using this.
* `send_mutually_recursive_message()`. If another thread is currently
* calling that function (from the UI thread), then we'll execute `f` from
* the UI thread using the IO context started in the above function.
* Otherwise `f` will be run on the UI thread through `main_context` as
* usual.
*
* @see Vst3Bridge::send_mutually_recursive_message
*/
template <typename T, typename F>
T do_mutual_recursion_on_gui_thread(F f) {
T do_mutual_recursion_or_handle_in_main_context(F f) {
std::packaged_task<T()> do_call(std::move(f));
std::future<T> do_call_response = do_call.get_future();
@@ -363,11 +339,10 @@ class Vst3Bridge : public HostBridge {
// `mutual_recursion_contexts` will be blocked until the deeper calls
// are finished.
{
std::lock_guard lock(gui_thread_mutual_recursion_contexts_mutex);
if (!gui_thread_mutual_recursion_contexts.empty()) {
boost::asio::dispatch(
*gui_thread_mutual_recursion_contexts.back(),
std::move(do_call));
std::lock_guard lock(mutual_recursion_contexts_mutex);
if (!mutual_recursion_contexts.empty()) {
boost::asio::dispatch(*mutual_recursion_contexts.back(),
std::move(do_call));
} else {
main_context.schedule_task(std::move(do_call));
}
@@ -377,24 +352,19 @@ class Vst3Bridge : public HostBridge {
}
/**
* The same as the above function, but for executing a function on some
* off-thread, that doesn't necessarily have to be the GUI thread. We
* separate these to make it impossible for these two things to potentially
* block eachother.
*
* You should pass `on_gui_thread = false` to
* `Vst3Bridge::send_mutually_recursive_message` when using this.
* The same as the above function, but we'll just execute the function on
* this thread when the mutual recursion context is not active.
*
* @see Vst3Bridge::do_mutual_recursion_or_handle_in_main_context
*/
template <typename T, typename F>
T do_mutual_recursion_on_off_thread(F f) {
T do_mutual_recursion(F f) {
std::packaged_task<T()> do_call(std::move(f));
std::future<T> do_call_response = do_call.get_future();
if (std::lock_guard lock(off_thread_mutual_recursion_contexts_mutex);
!off_thread_mutual_recursion_contexts.empty()) {
boost::asio::dispatch(*off_thread_mutual_recursion_contexts.back(),
if (std::lock_guard lock(mutual_recursion_contexts_mutex);
!mutual_recursion_contexts.empty()) {
boost::asio::dispatch(*mutual_recursion_contexts.back(),
std::move(do_call));
} else {
do_call();
@@ -508,22 +478,9 @@ class Vst3Bridge : public HostBridge {
* mutually recursive function calls. See the docstring there for more
* information. When this doesn't contain an IO context, this function is
* not being called and `do_mutual_recursion_or_handle_in_main_context()`
* should post the task directly to the main IO context, on the GUI thread.
* should post the task directly to the main IO context.
*/
std::vector<std::shared_ptr<boost::asio::io_context>>
gui_thread_mutual_recursion_contexts;
std::mutex gui_thread_mutual_recursion_contexts_mutex;
/**
* The same as `gui_thread_mutual_recursion_contexts`, but for tasks that
* can be executed on any thread as long as they're executed on the same
* thread. This is for instance necessary when the plugin tells the host
* that its latency has changed, and when the plugin gets reactivates by the
* host it will tell the host once again that its latency has changed. This
* would otherwise result in deadlocks with latency introducing JUCE VST3
* plugins under Ardour and Mixbus.
*/
std::vector<std::shared_ptr<boost::asio::io_context>>
off_thread_mutual_recursion_contexts;
std::mutex off_thread_mutual_recursion_contexts_mutex;
mutual_recursion_contexts;
std::mutex mutual_recursion_contexts_mutex;
};