diff --git a/meson.build b/meson.build index 0496849b..52247c46 100644 --- a/meson.build +++ b/meson.build @@ -454,6 +454,7 @@ if with_vst3 boost_dep, boost_filesystem_dep, bitsery_dep, + function2_dep, threads_dep, tomlplusplus_dep, vst3_sdk_native_dep, diff --git a/src/plugin/bridges/vst3-impls/plug-view-proxy.cpp b/src/plugin/bridges/vst3-impls/plug-view-proxy.cpp index 92c48090..8212ebdb 100644 --- a/src/plugin/bridges/vst3-impls/plug-view-proxy.cpp +++ b/src/plugin/bridges/vst3-impls/plug-view-proxy.cpp @@ -16,6 +16,73 @@ #include "plug-view-proxy.h" +RunLoopTasks::RunLoopTasks(Steinberg::IPtr plug_frame) + : run_loop(plug_frame) { + FUNKNOWN_CTOR + + if (!run_loop) { + throw std::runtime_error( + "The host's 'IPlugFrame' object does not support 'IRunLoop'"); + } + + // This should be backed by eventfd instead, but Ardour doesn't allow that + std::array sockets; + if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0, + sockets.data()) != 0) { + throw std::runtime_error("Failed to create a Unix domain socket"); + } + + socket_read_fd = sockets[0]; + socket_write_fd = sockets[1]; + if (run_loop->registerEventHandler(this, socket_read_fd) != + Steinberg::kResultOk) { + throw std::runtime_error( + "Failed to register an event handler with the host's run loop"); + } +} + +RunLoopTasks::~RunLoopTasks() { + FUNKNOWN_DTOR + + run_loop->unregisterEventHandler(this); + close(socket_read_fd); + close(socket_write_fd); +} + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdelete-non-virtual-dtor" +IMPLEMENT_FUNKNOWN_METHODS(RunLoopTasks, + Steinberg::Linux::IEventHandler, + Steinberg::Linux::IEventHandler::iid) +#pragma GCC diagnostic pop + +void RunLoopTasks::schedule(fu2::unique_function task) { + std::lock_guard eventfd_lock(tasks_mutex); + tasks.push_back(std::move(task)); + + uint8_t notify_value = 1; + write(socket_write_fd, ¬ify_value, sizeof(notify_value)); +} + +void PLUGIN_API +RunLoopTasks::onFDIsSet(Steinberg::Linux::FileDescriptor /*fd*/) { + std::lock_guard lock(tasks_mutex); + + // Run all tasks that have been submitted to our queue from the host's + // calling thread (which will be the GUI thread) + for (auto& task : tasks) { + task(); + + // This should in theory stop the host from calling this function, but + // REAPER doesn't care. And funnily enough we only have to do all of + // this because of REAPER. + uint8_t notify_value; + read(socket_read_fd, ¬ify_value, sizeof(notify_value)); + } + + tasks.clear(); +} + Vst3PlugViewProxyImpl::Vst3PlugViewProxyImpl( Vst3PluginBridge& bridge, Vst3PlugViewProxy::ConstructArgs&& args) @@ -24,7 +91,7 @@ Vst3PlugViewProxyImpl::Vst3PlugViewProxyImpl( Vst3PlugViewProxyImpl::~Vst3PlugViewProxyImpl() { // Also drop the plug view smart pointer on the Wine side when this gets // dropped - bridge.send_message( + send_mutually_recursive_message( Vst3PlugViewProxy::Destruct{.owner_instance_id = owner_instance_id()}); } @@ -42,8 +109,9 @@ Vst3PlugViewProxyImpl::isPlatformTypeSupported(Steinberg::FIDString type) { if (type) { // We'll swap the X11 window ID platform type string for the Win32 HWND // equivalent on the Wine side - return bridge.send_message(YaPlugView::IsPlatformTypeSupported{ - .owner_instance_id = owner_instance_id(), .type = type}); + return send_mutually_recursive_message( + YaPlugView::IsPlatformTypeSupported{ + .owner_instance_id = owner_instance_id(), .type = type}); } else { bridge.logger.log( "WARNING: Null pointer passed to " @@ -57,7 +125,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::attached(void* parent, if (parent && type) { // We will embed the Wine Win32 window into the X11 window provided by // the host - return bridge.send_message(YaPlugView::Attached{ + return send_mutually_recursive_message(YaPlugView::Attached{ .owner_instance_id = owner_instance_id(), .parent = reinterpret_cast(parent), .type = type}); @@ -69,19 +137,19 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::attached(void* parent, } tresult PLUGIN_API Vst3PlugViewProxyImpl::removed() { - return bridge.send_message( + return send_mutually_recursive_message( YaPlugView::Removed{.owner_instance_id = owner_instance_id()}); } tresult PLUGIN_API Vst3PlugViewProxyImpl::onWheel(float distance) { - return bridge.send_message(YaPlugView::OnWheel{ + return send_mutually_recursive_message(YaPlugView::OnWheel{ .owner_instance_id = owner_instance_id(), .distance = distance}); } tresult PLUGIN_API Vst3PlugViewProxyImpl::onKeyDown(char16 key, int16 keyCode, int16 modifiers) { - return bridge.send_message( + return send_mutually_recursive_message( YaPlugView::OnKeyDown{.owner_instance_id = owner_instance_id(), .key = key, .key_code = keyCode, @@ -91,7 +159,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::onKeyDown(char16 key, tresult PLUGIN_API Vst3PlugViewProxyImpl::onKeyUp(char16 key, int16 keyCode, int16 modifiers) { - return bridge.send_message( + return send_mutually_recursive_message( YaPlugView::OnKeyUp{.owner_instance_id = owner_instance_id(), .key = key, .key_code = keyCode, @@ -101,7 +169,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::onKeyUp(char16 key, tresult PLUGIN_API Vst3PlugViewProxyImpl::getSize(Steinberg::ViewRect* size) { if (size) { const GetSizeResponse response = - bridge.send_message(YaPlugView::GetSize{ + send_mutually_recursive_message(YaPlugView::GetSize{ .owner_instance_id = owner_instance_id(), .size = *size}); *size = response.updated_size; @@ -116,7 +184,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::getSize(Steinberg::ViewRect* size) { tresult PLUGIN_API Vst3PlugViewProxyImpl::onSize(Steinberg::ViewRect* newSize) { if (newSize) { - return bridge.send_message(YaPlugView::OnSize{ + return send_mutually_recursive_message(YaPlugView::OnSize{ .owner_instance_id = owner_instance_id(), .new_size = *newSize}); } else { bridge.logger.log( @@ -126,7 +194,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::onSize(Steinberg::ViewRect* newSize) { } tresult PLUGIN_API Vst3PlugViewProxyImpl::onFocus(TBool state) { - return bridge.send_message(YaPlugView::OnFocus{ + return send_mutually_recursive_message(YaPlugView::OnFocus{ .owner_instance_id = owner_instance_id(), .state = state}); } @@ -138,7 +206,25 @@ Vst3PlugViewProxyImpl::setFrame(Steinberg::IPlugFrame* frame) { // this component handler plug_frame = frame; - return bridge.send_message(YaPlugView::SetFrame{ + // REAPER's GUI is not thread safe, and if we don't call + // `IPlugFrame::resizeView()` or `IContextMenu::popup()` from a thread + // owned by REAPER then REAPER will eventually segfault We should thus + // try to call those functions from an `IRunLoop` event handler. + try { + run_loop_tasks = Steinberg::owned(new RunLoopTasks(plug_frame)); + } catch (const std::runtime_error& e) { + // In case the host does not support `IRunLoop` or if we can't + // register an event handler, we'll throw during `RunLoopTasks`' + // constructor + run_loop_tasks = nullptr; + + bridge.logger.log( + "The host does not support IRunLoop, falling back to naive GUI " + "function execution: " + + std::string(e.what())); + } + + return send_mutually_recursive_message(YaPlugView::SetFrame{ .owner_instance_id = owner_instance_id(), .plug_frame_args = Vst3PlugFrameProxy::ConstructArgs( plug_frame, owner_instance_id())}); @@ -150,7 +236,7 @@ Vst3PlugViewProxyImpl::setFrame(Steinberg::IPlugFrame* frame) { } tresult PLUGIN_API Vst3PlugViewProxyImpl::canResize() { - return bridge.send_message( + return send_mutually_recursive_message( YaPlugView::CanResize{.owner_instance_id = owner_instance_id()}); } @@ -158,7 +244,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::checkSizeConstraint(Steinberg::ViewRect* rect) { if (rect) { const CheckSizeConstraintResponse response = - bridge.send_message(YaPlugView::CheckSizeConstraint{ + send_mutually_recursive_message(YaPlugView::CheckSizeConstraint{ .owner_instance_id = owner_instance_id(), .rect = *rect}); *rect = response.updated_rect; @@ -177,7 +263,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::findParameter( int32 yPos, Steinberg::Vst::ParamID& resultTag /*out*/) { const FindParameterResponse response = - bridge.send_message(YaParameterFinder::FindParameter{ + send_mutually_recursive_message(YaParameterFinder::FindParameter{ .owner_instance_id = owner_instance_id(), .x_pos = xPos, .y_pos = yPos}); @@ -189,7 +275,7 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::findParameter( tresult PLUGIN_API Vst3PlugViewProxyImpl::setContentScaleFactor(ScaleFactor factor) { - return bridge.send_message( + return send_mutually_recursive_message( YaPlugViewContentScaleSupport::SetContentScaleFactor{ .owner_instance_id = owner_instance_id(), .factor = factor}); } diff --git a/src/plugin/bridges/vst3-impls/plug-view-proxy.h b/src/plugin/bridges/vst3-impls/plug-view-proxy.h index 33229dae..61f477c4 100644 --- a/src/plugin/bridges/vst3-impls/plug-view-proxy.h +++ b/src/plugin/bridges/vst3-impls/plug-view-proxy.h @@ -16,8 +16,93 @@ #pragma once +#include + #include "../vst3.h" +#include + +/** + * A RAII wrapper around `IRunLoop`'s event handlers so we can schedule tasks to + * be run in it. This is needed for REAPER, because function calls that involve + * GUI drawing (notable `IPlugFrame::resizeView()` and `IContextMenu::popup()`) + * have to be run from a thread owned by REAPER. If we don't do this, the + * `IPlugFrame::resizeView()` won't resize the actual window and both of these + * functions will eventually cause REAPER to segfault. + */ +class RunLoopTasks : public Steinberg::Linux::IEventHandler { + public: + /** + * Register an event handler in the host's run loop so we can schedule tasks + * to be run from there. This works very much like how we use Boost.Asio IO + * contexts everywhere else to run functions on other threads. All of this + * is backed by a dummy Unix domain socket, although REAPER will call the + * event handler regardless of whether the file descriptor is ready or not. + * eventfd would have made much more sense here, but Ardour doesn't support + * that. + * + * @throw std::runtime_error If the host does not support + * `Steinberg::Linux::IRunLoop`, or if registering the event handler was + * not successful. The caller should catch this and call back to not + * relying on the run loop. + */ + RunLoopTasks(Steinberg::IPtr plug_frame); + + /** + * Unregister the event handler and close the file descriptor on cleanup. + */ + ~RunLoopTasks(); + + DECLARE_FUNKNOWN_METHODS + + /** + * Schedule a task to be run from the host's GUI thread in an `IRunLoop` + * event handler. This may block if the host is currently calling + * `onFDIsSet()`. + * + * @param task The task to execute. This can be used with + * `std::packaged_task` to run a computation that returns a value from the + * host's GUI thread. + */ + void schedule(fu2::unique_function task); + + // From `IEventHandler`, required for REAPER because its GUI is not thread + // safe + void PLUGIN_API onFDIsSet(Steinberg::Linux::FileDescriptor fd) override; + + private: + /** + * This pointer is cast from `plug_frame` once `IPlugView::setFrame()` has + * been called. + */ + Steinberg::FUnknownPtr run_loop; + + /** + * Tasks that should be executed in the next `IRunLoop` event handler call. + * + * @relates Vst3PlugViewProxyImpl::run_gui_task + * + * @see RunLoopTasks::schedule_task + */ + std::vector> tasks; + std::mutex tasks_mutex; + + /** + * A dumy Unix domain socket file descriptor used to signal that there is a + * task ready. We'll pass this to the host's `IRunLoop` so it can tell when + * we have an event to handle. + * + * XXX: This should be backed by eventfd instead, but Ardour doesn't support + * that + */ + int socket_read_fd = -1; + /** + * The other side of `socket_read_fd`. We'll write to this when we want the + * hsot to call our event handler. + */ + int socket_write_fd = -1; +}; + class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy { public: Vst3PlugViewProxyImpl(Vst3PluginBridge& bridge, @@ -37,6 +122,55 @@ class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy { tresult PLUGIN_API queryInterface(const Steinberg::TUID _iid, void** obj) override; + /** + * Run a task that's supposed to be run from the GUI thread. + * `IPlugFrame::resizeView()` and `IContextMenu::popup()` are the likely + * candidates here. This is needed for REAPER, as REAPER will segfault if + * you run those functions from a thread that's not owned by REAPER itself. + * If the `IPlugFrame` object passed to `IPlugView::setFrame()` supports + * `IRunLoop`, then we'll schedule `f` to be run from an even handler in the + * host's run loop. Otherwise `f` is run directly. + * + * This works similarly to + * `Vst3Bridge::do_mutual_recursion_or_handle_in_main_context`, except that + * we can post tasks to `run_loop_tasks` instead of executing them directly + * in `main_context` when no mutually recursive function calls are happening + * right now. + * + * @see send_mutually_recursive_message + */ + template + T run_gui_task(F f) { + std::packaged_task do_call(std::move(f)); + std::future do_call_response = do_call.get_future(); + + // If `send_mutually_recursive_message()` is currently being called + // (because the host is calling one of `IPlugView`'s methods from its + // UGI thread) then we'll post a message to an IO context that's + // currently accepting work on the that thread. Otherwise we'll schedule + // the task to be run from an event handler registered to the host's run + // loop. If the host does not support `IRunLoop`, we'll just run `f` + // directly. + { + std::unique_lock mutual_recursion_lock( + mutual_recursion_context_mutex); + if (mutual_recursion_context) { + boost::asio::dispatch(*mutual_recursion_context, + std::move(do_call)); + } else { + mutual_recursion_lock.unlock(); + + if (run_loop_tasks) { + run_loop_tasks->schedule(std::move(do_call)); + } else { + do_call(); + } + } + } + + return do_call_response.get(); + } + // From `IPlugView` tresult PLUGIN_API isPlatformTypeSupported(Steinberg::FIDString type) override; @@ -74,5 +208,85 @@ class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy { Steinberg::IPtr plug_frame; private: + /** + * Send a message from this `IPlugView` instance. This function will be + * called by the host on its GUI thread, so until this function returns + * we'll know that the no `IRunLoop` event handlers will be called. Because + * of this we'll have to use this function to handling mutually recursive + * function calls, such as the calling sequence for resizing views. This + * should be used instead of sending the messages directly. + * + * We use the same trick in `Vst3Bridge`. + */ + template + typename T::Response send_mutually_recursive_message(const T& object) { + using TResponse = typename T::Response; + + // This IO context will accept incoming calls from `run_gui_task()` + // until we receive a response + { + std::unique_lock lock(mutual_recursion_context_mutex); + + // In case some other thread is already calling + // `send_mutually_recursive_message()`, we're likely in mutually + // recursive calling sequence, and we should thus run the message + // from the current thread. This can happen during + // `IPlugView::attached() -> IPlugFrame::resizeView() -> + // IPlugView::onSize()`. + if (mutual_recursion_context) { + lock.unlock(); + + return bridge.send_message(object); + } + + mutual_recursion_context.emplace(); + } + + // We will call the function from another thread so we can handle calls + // to from this thread + std::promise response_promise{}; + std::jthread sending_thread([&]() { + const TResponse response = bridge.send_message(object); + + // Stop accepting additional work to be run from the calling thread + // once we receive a response + std::lock_guard lock(mutual_recursion_context_mutex); + mutual_recursion_context->stop(); + mutual_recursion_context.reset(); + + response_promise.set_value(response); + }); + + // 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(*mutual_recursion_context); + mutual_recursion_context->run(); + + return response_promise.get_future().get(); + } + Vst3PluginBridge& bridge; + + /** + * The IO context used in `send_mutually_recursive_message()` to be able to + * execute functions from that same calling thread while we're waiting for a + * response. See the docstring there for more information. When this doesn't + * contain an IO context, this function is not being called and + * `run_gui_task()` should post the task to `run_loop_tasks`. This works + * exactly the same as the mutual recursion handling in `Vst3Bridge`. + */ + std::optional mutual_recursion_context; + std::mutex mutual_recursion_context_mutex; + + /** + * If the host supports `IRunLoop`, we'll use this to run certain tasks from + * the host's GUI thread using a run loop event handler in + * `Vst3PlugViewProxyImpl::run_gui_task`. + * + * _This value is optional_ and it will this be a null pointer of the host + * does not support `IRunLoop`. We have to use an `IPtr` instead of a an + * `std::optional` in case the host also stores a pointer to this. + */ + Steinberg::IPtr run_loop_tasks; }; diff --git a/src/plugin/bridges/vst3.cpp b/src/plugin/bridges/vst3.cpp index 2234c53b..2d4fef01 100644 --- a/src/plugin/bridges/vst3.cpp +++ b/src/plugin/bridges/vst3.cpp @@ -206,20 +206,16 @@ Vst3PluginBridge::Vst3PluginBridge() }, [&](const YaContextMenu::Popup& request) -> YaContextMenu::Popup::Response { - // FIXME: In REAPER having the menu open without interacting - // with it causes malloc failures or failing font - // drawing calls. Valgrind reports all kinds of - // memory errors within REAPER when this happens, and - // I'm not sure if yabridge is to blame here. - As it - // turns out a lot of stuff in REAPEr, including - // calls to `IPlugFrame::resizeView()`, are not - // thread safe. We need to hook into `IRunLoop` and - // execute `IContextMenu::popup()` and - // `IPlugFrame::resizeView()` functions from there. + // REAPER requires this to be run from its provided event + // loop or else it will likely segfault at some point return plugin_proxies.at(request.owner_instance_id) .get() - .context_menus.at(request.context_menu_id) - .menu->popup(request.x, request.y); + .last_created_plug_view->run_gui_task([&]() { + return plugin_proxies.at(request.owner_instance_id) + .get() + .context_menus.at(request.context_menu_id) + .menu->popup(request.x, request.y); + }); }, [&](YaConnectionPoint::Notify& request) -> YaConnectionPoint::Notify::Response { @@ -267,8 +263,12 @@ Vst3PluginBridge::Vst3PluginBridge() .get() .last_created_plug_view; - return plug_view->plug_frame->resizeView(plug_view, - &request.new_size); + // REAPER requires this to be run from its provided event + // loop or else it will likely segfault at some point + return plug_view->run_gui_task([&]() { + return plug_view->plug_frame->resizeView( + plug_view, &request.new_size); + }); }, [&](const YaPlugInterfaceSupport::IsPlugInterfaceSupported& request) -> YaPlugInterfaceSupport:: diff --git a/src/wine-host/bridges/vst3.h b/src/wine-host/bridges/vst3.h index def9fdfd..3add9699 100644 --- a/src/wine-host/bridges/vst3.h +++ b/src/wine-host/bridges/vst3.h @@ -244,6 +244,8 @@ class Vst3Bridge : public HostBridge { * The hacky solution here is to send the message from another thread, and * to then allow this thread to execute other functions submitted to an IO * context. + * + * We apply the same trick in `Vst3PlugViewProxyImpl`. */ template typename T::Response send_mutually_recursive_message(const T& object) {