From a9643577fd2593b06f950a7e13bee72e8f0b645e Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 14 May 2021 18:27:19 +0200 Subject: [PATCH] Also add noexcept qualifications on the Wine side See the last few commits. --- src/wine-host/bridges/common.cpp | 4 +- src/wine-host/bridges/common.h | 6 +-- src/wine-host/bridges/group.cpp | 6 +-- src/wine-host/bridges/group.h | 18 ++++++--- src/wine-host/bridges/vst2.cpp | 7 ++-- src/wine-host/bridges/vst2.h | 4 +- src/wine-host/bridges/vst3.cpp | 14 +++---- src/wine-host/bridges/vst3.h | 14 +++---- src/wine-host/editor.cpp | 66 +++++++++++++++++--------------- src/wine-host/editor.h | 10 ++--- src/wine-host/utils.cpp | 41 ++++++++++---------- src/wine-host/utils.h | 28 +++++++------- 12 files changed, 118 insertions(+), 100 deletions(-) diff --git a/src/wine-host/bridges/common.cpp b/src/wine-host/bridges/common.cpp index 57b614d4..5cd06d8c 100644 --- a/src/wine-host/bridges/common.cpp +++ b/src/wine-host/bridges/common.cpp @@ -29,7 +29,9 @@ HostBridge::HostBridge(MainContext& main_context, parent_pid(parent_pid), watchdog_guard(main_context.register_watchdog(*this)) {} -void HostBridge::handle_win32_events() { +HostBridge::~HostBridge() noexcept {} + +void HostBridge::handle_win32_events() noexcept { MSG msg; for (int i = 0; diff --git a/src/wine-host/bridges/common.h b/src/wine-host/bridges/common.h index 14f08a77..74d5a946 100644 --- a/src/wine-host/bridges/common.h +++ b/src/wine-host/bridges/common.h @@ -37,7 +37,7 @@ class HostBridge { pid_t parent_pid); public: - virtual ~HostBridge(){}; + virtual ~HostBridge() noexcept; /** * If a plugin instance returns `true` here, then the event loop should not @@ -51,7 +51,7 @@ class HostBridge { * * @relates MainContext::async_handle_events */ - virtual bool inhibits_event_loop() = 0; + virtual bool inhibits_event_loop() noexcept = 0; /** * Handle events until the plugin exits. The actual events are posted to @@ -88,7 +88,7 @@ class HostBridge { * because of incorrect assumptions made by the plugin. See the dostring for * `Vst2Bridge::editor` for more information. */ - void handle_win32_events(); + void handle_win32_events() noexcept; /** * Used as part of the watchdog. This will check whether the remote host diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index 9bb9d9d2..89cb268e 100644 --- a/src/wine-host/bridges/group.cpp +++ b/src/wine-host/bridges/group.cpp @@ -78,7 +78,7 @@ StdIoCapture::StdIoCapture(boost::asio::io_context& io_context, pipe.assign(pipe_fd[0]); } -StdIoCapture::~StdIoCapture() { +StdIoCapture::~StdIoCapture() noexcept { // Restore the original file descriptor and close the pipe. The other wend // was already closed in the constructor. dup2(original_fd_copy, target_fd); @@ -112,11 +112,11 @@ GroupBridge::GroupBridge(boost::filesystem::path group_socket_path) }); } -GroupBridge::~GroupBridge() { +GroupBridge::~GroupBridge() noexcept { stdio_context.stop(); } -bool GroupBridge::is_event_loop_inhibited() { +bool GroupBridge::is_event_loop_inhibited() noexcept { std::lock_guard lock(active_plugins_mutex); for (auto& [parameters, value] : active_plugins) { diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h index fd5ff5b9..f30bec91 100644 --- a/src/wine-host/bridges/group.h +++ b/src/wine-host/bridges/group.h @@ -51,14 +51,17 @@ class StdIoCapture { */ StdIoCapture(boost::asio::io_context& io_context, int file_descriptor); - StdIoCapture(const StdIoCapture&) = delete; - StdIoCapture& operator=(const StdIoCapture&) = delete; - /** * On cleanup, close the outgoing file descriptor from the pipe and restore * the original file descriptor for the captured stream. */ - ~StdIoCapture(); + ~StdIoCapture() noexcept; + + StdIoCapture(const StdIoCapture&) = delete; + StdIoCapture& operator=(const StdIoCapture&) = delete; + + StdIoCapture(StdIoCapture&&) = delete; + StdIoCapture& operator=(StdIoCapture&&) = delete; /** * The pipe endpoint where all output from the original file descriptor gets @@ -122,18 +125,21 @@ class GroupBridge { */ explicit GroupBridge(boost::filesystem::path group_socket_path); - ~GroupBridge(); + ~GroupBridge() noexcept; GroupBridge(const GroupBridge&) = delete; GroupBridge& operator=(const GroupBridge&) = delete; + GroupBridge(const GroupBridge&&) = delete; + GroupBridge& operator=(GroupBridge&&) = delete; + /** * If this returns `true`, then the group host's event loop should * temporarily be disabled. This simply calls * `HostBridge::inhibits_event_loop()` for all plugins hosted in this group * process. */ - bool is_event_loop_inhibited(); + bool is_event_loop_inhibited() noexcept; /** * Run a plugin's dispatcher and message loop, processing all events on the diff --git a/src/wine-host/bridges/vst2.cpp b/src/wine-host/bridges/vst2.cpp index 2273be39..c6e69e69 100644 --- a/src/wine-host/bridges/vst2.cpp +++ b/src/wine-host/bridges/vst2.cpp @@ -316,7 +316,7 @@ Vst2Bridge::Vst2Bridge(MainContext& main_context, }); } -bool Vst2Bridge::inhibits_event_loop() { +bool Vst2Bridge::inhibits_event_loop() noexcept { return !is_initialized; } @@ -420,7 +420,7 @@ void Vst2Bridge::run() { }); } -void Vst2Bridge::handle_x11_events() { +void Vst2Bridge::handle_x11_events() noexcept { if (editor) { editor->handle_x11_events(); } @@ -488,7 +488,8 @@ intptr_t Vst2Bridge::dispatch_wrapper(AEffect* plugin, class HostCallbackDataConverter : DefaultDataConverter { public: - HostCallbackDataConverter(AEffect* plugin, VstTimeInfo& last_time_info) + HostCallbackDataConverter(AEffect* plugin, + VstTimeInfo& last_time_info) noexcept : plugin(plugin), last_time_info(last_time_info) {} EventPayload read(const int opcode, diff --git a/src/wine-host/bridges/vst2.h b/src/wine-host/bridges/vst2.h index c8fcb8c9..daf837ea 100644 --- a/src/wine-host/bridges/vst2.h +++ b/src/wine-host/bridges/vst2.h @@ -58,7 +58,7 @@ class Vst2Bridge : public HostBridge { std::string endpoint_base_dir, pid_t parent_pid); - bool inhibits_event_loop() override; + bool inhibits_event_loop() noexcept override; /** * Here we'll handle incoming `dispatch()` messages until the sockets get @@ -66,7 +66,7 @@ class Vst2Bridge : public HostBridge { */ void run() override; - void handle_x11_events() override; + void handle_x11_events() noexcept override; protected: void close_sockets() override; diff --git a/src/wine-host/bridges/vst3.cpp b/src/wine-host/bridges/vst3.cpp index 8a49455c..ff0852c9 100644 --- a/src/wine-host/bridges/vst3.cpp +++ b/src/wine-host/bridges/vst3.cpp @@ -38,18 +38,18 @@ Steinberg::FUnknownPtr hack_init_plugin_base( Steinberg::IPtr object, Steinberg::IPtr component); -InstancePlugView::InstancePlugView() {} +InstancePlugView::InstancePlugView() noexcept {} InstancePlugView::InstancePlugView( - Steinberg::IPtr plug_view) + Steinberg::IPtr plug_view) noexcept : plug_view(plug_view), parameter_finder(plug_view), plug_view_content_scale_support(plug_view) {} -InstanceInterfaces::InstanceInterfaces() {} +InstanceInterfaces::InstanceInterfaces() noexcept {} InstanceInterfaces::InstanceInterfaces( - Steinberg::IPtr object) + Steinberg::IPtr object) noexcept : object(object), audio_presentation_latency(object), audio_processor(object), @@ -119,7 +119,7 @@ Vst3Bridge::Vst3Bridge(MainContext& main_context, main_context.update_timer_interval(config.event_loop_interval()); } -bool Vst3Bridge::inhibits_event_loop() { +bool Vst3Bridge::inhibits_event_loop() noexcept { std::lock_guard lock(object_instances_mutex); for (const auto& [instance_id, object] : object_instances) { @@ -1121,7 +1121,7 @@ void Vst3Bridge::run() { }); } -void Vst3Bridge::handle_x11_events() { +void Vst3Bridge::handle_x11_events() noexcept { std::lock_guard lock(object_instances_mutex); for (const auto& [instance_id, object] : object_instances) { @@ -1154,7 +1154,7 @@ void Vst3Bridge::unregister_context_menu(size_t object_instance_id, context_menu_id); } -size_t Vst3Bridge::generate_instance_id() { +size_t Vst3Bridge::generate_instance_id() noexcept { return current_instance_id.fetch_add(1); } diff --git a/src/wine-host/bridges/vst3.h b/src/wine-host/bridges/vst3.h index edebe9d5..74b91291 100644 --- a/src/wine-host/bridges/vst3.h +++ b/src/wine-host/bridges/vst3.h @@ -36,9 +36,9 @@ class Vst3ContextMenuProxyImpl; * @relates InstanceInterfaces */ struct InstancePlugView { - InstancePlugView(); + InstancePlugView() noexcept; - InstancePlugView(Steinberg::IPtr plug_View); + InstancePlugView(Steinberg::IPtr plug_View) noexcept; Steinberg::IPtr plug_view; @@ -59,9 +59,9 @@ struct InstancePlugView { * `IPluginBase::initialize()`. */ struct InstanceInterfaces { - InstanceInterfaces(); + InstanceInterfaces() noexcept; - InstanceInterfaces(Steinberg::IPtr object); + InstanceInterfaces(Steinberg::IPtr object) noexcept; /** * A dedicated thread for handling incoming `IAudioProcessor` and @@ -227,7 +227,7 @@ class Vst3Bridge : public HostBridge { * `object_instances` that supports `IPluginBase` whether * `IPluginBase::iniitalize()` has been called. */ - bool inhibits_event_loop() override; + bool inhibits_event_loop() noexcept override; /** * Here we'll listen for and handle incoming control messages until the @@ -235,7 +235,7 @@ class Vst3Bridge : public HostBridge { */ void run() override; - void handle_x11_events() override; + void handle_x11_events() noexcept override; protected: void close_sockets() override; @@ -416,7 +416,7 @@ class Vst3Bridge : public HostBridge { * is used to be able to refer to specific instances created for * `IPluginFactory::createInstance()`. */ - size_t generate_instance_id(); + size_t generate_instance_id() noexcept; /** * Assign a unique identifier to an object and add it to `object_instances`. diff --git a/src/wine-host/editor.cpp b/src/wine-host/editor.cpp index 664057e1..0b7225dd 100644 --- a/src/wine-host/editor.cpp +++ b/src/wine-host/editor.cpp @@ -110,7 +110,7 @@ bool is_child_window_or_same(xcb_connection_t& x11_connection, * Compute the size a window would have to be to be allowed to fullscreened on * any of the connected screens. */ -Size get_maximum_screen_dimensions(xcb_connection_t& x11_connection); +Size get_maximum_screen_dimensions(xcb_connection_t& x11_connection) noexcept; /** * Get the root window for the specified window. The returned root window will * depend on the screen the window is on. @@ -120,22 +120,22 @@ xcb_window_t get_root_window(xcb_connection_t& x11_connection, /** * Return the X11 window handle for the window if it's currently open. */ -xcb_window_t get_x11_handle(HWND win32_handle); +xcb_window_t get_x11_handle(HWND win32_handle) noexcept; /** * Return a handle to the window class used for all Win32 windows created by * yabridge. */ -ATOM get_window_class(); +ATOM get_window_class() noexcept; DeferredWindow::DeferredWindow(MainContext& main_context, std::shared_ptr x11_connection, - HWND window) + HWND window) noexcept : handle(window), main_context(main_context), x11_connection(x11_connection) {} -DeferredWindow::~DeferredWindow() { +DeferredWindow::~DeferredWindow() noexcept { // NOTE: For some rason, Wine will sometimes try to delete a window twice if // the parent window no longer exists. I've only seen this cause // issues with plugins that hang when their window is hidden, like the @@ -159,28 +159,34 @@ DeferredWindow::~DeferredWindow() { // would tick exactly between `IPlugView::removed()` and // `IPlugView::~IPlugView`. Delaying this seems to be a best of both // worlds solution that works as expected in every host I've tested. - std::shared_ptr destroy_timer = - std::make_shared(main_context.context); - destroy_timer->expires_after(1s); + try { + std::shared_ptr destroy_timer = + std::make_shared(main_context.context); + destroy_timer->expires_after(1s); - // Note that we capture a copy of `destroy_timer` here. This way we don't - // have to manage the timer instance ourselves as it will just clean itself - // up after this lambda gets called. - destroy_timer->async_wait([destroy_timer, handle = this->handle, - x11_connection = this->x11_connection]( - const boost::system::error_code& error) { - if (error.failed()) { - return; - } + // Note that we capture a copy of `destroy_timer` here. This way we + // don't have to manage the timer instance ourselves as it will just + // clean itself up after this lambda gets called. + destroy_timer->async_wait([destroy_timer, handle = this->handle, + x11_connection = this->x11_connection]( + const boost::system::error_code& error) { + if (error.failed()) { + return; + } - // This is the flush for the reparent done above. We'll also do this as - // late as possible to prevent the window from being drawn in the - // meantime, as that would cause flickering. - xcb_flush(x11_connection.get()); + // This is the flush for the reparent done above. We'll also do this + // as late as possible to prevent the window from being drawn in the + // meantime, as that would cause flickering. + xcb_flush(x11_connection.get()); - // The actual destroying will happen as part of the Win32 message loop - PostMessage(handle, WM_CLOSE, 0, 0); - }); + // The actual destroying will happen as part of the Win32 message + // loop + PostMessage(handle, WM_CLOSE, 0, 0); + }); + } catch (const std::bad_alloc&) { + // If we can't allocate the timer, then we probably have bigger worries + // than not cleaning up a window + } } Editor::Editor(MainContext& main_context, @@ -352,7 +358,7 @@ Editor::Editor(MainContext& main_context, } } -HWND Editor::get_win32_handle() const { +HWND Editor::get_win32_handle() const noexcept { // FIXME: The double embed and XEmbed options don't work together right now if (win32_child_window && !use_xembed) { return win32_child_window->handle; @@ -361,7 +367,7 @@ HWND Editor::get_win32_handle() const { } } -void Editor::handle_x11_events() const { +void Editor::handle_x11_events() const noexcept { // NOTE: Ardour will unmap the window instead of closing the editor. When // the window is unmapped `wine_window` doesn't exist and any X11 // function calls involving it will fail. All functions called from @@ -623,7 +629,7 @@ void Editor::send_xembed_message(const xcb_window_t& window, const uint32_t message, const uint32_t detail, const uint32_t data1, - const uint32_t data2) const { + const uint32_t data2) const noexcept { xcb_client_message_event_t event; event.response_type = XCB_CLIENT_MESSAGE; event.type = xcb_xembed_message; @@ -793,7 +799,7 @@ bool is_child_window_or_same(xcb_connection_t& x11_connection, return false; } -Size get_maximum_screen_dimensions(xcb_connection_t& x11_connection) { +Size get_maximum_screen_dimensions(xcb_connection_t& x11_connection) noexcept { xcb_screen_iterator_t iter = xcb_setup_roots_iterator(xcb_get_setup(&x11_connection)); @@ -829,12 +835,12 @@ xcb_window_t get_root_window(xcb_connection_t& x11_connection, return root; } -xcb_window_t get_x11_handle(HWND win32_handle) { +xcb_window_t get_x11_handle(HWND win32_handle) noexcept { return reinterpret_cast( GetProp(win32_handle, "__wine_x11_whole_window")); } -ATOM get_window_class() { +ATOM get_window_class() noexcept { // Lazily iniitialize our window class static ATOM window_class_handle = 0; if (!window_class_handle) { diff --git a/src/wine-host/editor.h b/src/wine-host/editor.h index 56532b65..7b659a62 100644 --- a/src/wine-host/editor.h +++ b/src/wine-host/editor.h @@ -80,13 +80,13 @@ class DeferredWindow { */ DeferredWindow(MainContext& main_context, std::shared_ptr x11_connection, - HWND window); + HWND window) noexcept; /** * Post a `WM_CLOSE` message to the `handle`'s message queue as described * above. */ - ~DeferredWindow(); + ~DeferredWindow() noexcept; const HWND handle; @@ -145,7 +145,7 @@ class Editor { * call. This will return the child window's handle if double editor * embedding is enabled. */ - HWND get_win32_handle() const; + HWND get_win32_handle() const noexcept; /** * Returns `true` if the window manager supports the EWMH active window @@ -161,7 +161,7 @@ class Editor { /** * Handle X11 events sent to the window our editor is embedded in. */ - void handle_x11_events() const; + void handle_x11_events() const noexcept; /** * Lie to the Wine window about its coordinates on the screen for @@ -215,7 +215,7 @@ class Editor { const uint32_t message, const uint32_t detail, const uint32_t data1, - const uint32_t data2) const; + const uint32_t data2) const noexcept; /** * Start the XEmbed procedure when `use_xembed` is enabled. This should be diff --git a/src/wine-host/utils.cpp b/src/wine-host/utils.cpp index 659aa8d3..da01cb7d 100644 --- a/src/wine-host/utils.cpp +++ b/src/wine-host/utils.cpp @@ -28,46 +28,47 @@ win32_thread_trampoline(fu2::unique_function* entry_point) { return 0; } -Win32Thread::Win32Thread(Win32Thread&& o) : handle(std::move(o.handle)) { +Win32Thread::Win32Thread() noexcept : handle(nullptr, nullptr) {} + +Win32Thread::~Win32Thread() noexcept { + if (handle) { + WaitForSingleObject(handle.get(), INFINITE); + } +} + +Win32Thread::Win32Thread(Win32Thread&& o) noexcept + : handle(std::move(o.handle)) { o.handle.reset(); } -Win32Thread& Win32Thread::operator=(Win32Thread&& o) { +Win32Thread& Win32Thread::operator=(Win32Thread&& o) noexcept { handle = std::move(o.handle); o.handle.reset(); return *this; } -Win32Thread::~Win32Thread() { - if (handle) { - WaitForSingleObject(handle.get(), INFINITE); - } -} - -Win32Thread::Win32Thread() : handle(nullptr, nullptr) {} - -Win32Timer::Win32Timer() {} +Win32Timer::Win32Timer() noexcept {} Win32Timer::Win32Timer(HWND window_handle, size_t timer_id, - unsigned int interval_ms) + unsigned int interval_ms) noexcept : window_handle(window_handle), timer_id(timer_id) { SetTimer(window_handle, timer_id, interval_ms, nullptr); } -Win32Timer::~Win32Timer() { +Win32Timer::~Win32Timer() noexcept { if (timer_id) { KillTimer(window_handle, *timer_id); } } -Win32Timer::Win32Timer(Win32Timer&& o) +Win32Timer::Win32Timer(Win32Timer&& o) noexcept : window_handle(o.window_handle), timer_id(std::move(o.timer_id)) { o.timer_id.reset(); } -Win32Timer& Win32Timer::operator=(Win32Timer&& o) { +Win32Timer& Win32Timer::operator=(Win32Timer&& o) noexcept { window_handle = o.window_handle; timer_id = std::move(o.timer_id); o.timer_id.reset(); @@ -95,12 +96,12 @@ void MainContext::run() { context.run(); } -void MainContext::stop() { +void MainContext::stop() noexcept { context.stop(); } void MainContext::update_timer_interval( - std::chrono::steady_clock::duration new_interval) { + std::chrono::steady_clock::duration new_interval) noexcept { timer_interval = new_interval; } @@ -115,14 +116,14 @@ MainContext::WatchdogGuard::WatchdogGuard( watched_bridges.insert(&bridge); } -MainContext::WatchdogGuard::~WatchdogGuard() { +MainContext::WatchdogGuard::~WatchdogGuard() noexcept { if (is_active) { std::lock_guard lock(watched_bridges_mutex.get()); watched_bridges.get().erase(bridge); } } -MainContext::WatchdogGuard::WatchdogGuard(WatchdogGuard&& o) +MainContext::WatchdogGuard::WatchdogGuard(WatchdogGuard&& o) noexcept : bridge(std::move(o.bridge)), watched_bridges(std::move(o.watched_bridges)), watched_bridges_mutex(std::move(o.watched_bridges_mutex)) { @@ -130,7 +131,7 @@ MainContext::WatchdogGuard::WatchdogGuard(WatchdogGuard&& o) } MainContext::WatchdogGuard& MainContext::WatchdogGuard::operator=( - WatchdogGuard&& o) { + WatchdogGuard&& o) noexcept { bridge = std::move(o.bridge); watched_bridges = std::move(o.watched_bridges); watched_bridges_mutex = std::move(o.watched_bridges_mutex); diff --git a/src/wine-host/utils.h b/src/wine-host/utils.h index fbb58c47..3bfe4f91 100644 --- a/src/wine-host/utils.h +++ b/src/wine-host/utils.h @@ -70,7 +70,7 @@ class Win32Thread { /** * Constructor that does not start any thread yet. */ - Win32Thread(); + Win32Thread() noexcept; /** * Constructor that immediately starts running the thread. This works @@ -109,8 +109,8 @@ class Win32Thread { Win32Thread(const Win32Thread&) = delete; Win32Thread& operator=(const Win32Thread&) = delete; - Win32Thread(Win32Thread&&); - Win32Thread& operator=(Win32Thread&&); + Win32Thread(Win32Thread&&) noexcept; + Win32Thread& operator=(Win32Thread&&) noexcept; private: // FIXME: This emits `-Wignored-attributes` as of Wine 5.22 @@ -133,16 +133,18 @@ class Win32Thread { */ class Win32Timer { public: - Win32Timer(); - Win32Timer(HWND window_handle, size_t timer_id, unsigned int interval_ms); + Win32Timer() noexcept; + Win32Timer(HWND window_handle, + size_t timer_id, + unsigned int interval_ms) noexcept; - ~Win32Timer(); + ~Win32Timer() noexcept; Win32Timer(const Win32Timer&) = delete; Win32Timer& operator=(const Win32Timer&) = delete; - Win32Timer(Win32Timer&&); - Win32Timer& operator=(Win32Timer&&); + Win32Timer(Win32Timer&&) noexcept; + Win32Timer& operator=(Win32Timer&&) noexcept; private: HWND window_handle; @@ -176,7 +178,7 @@ class MainContext { * Drop all future work from the IO context. This does not necessarily mean * that the thread that called `main_context.run()` immediatly returns. */ - void stop(); + void stop() noexcept; /** * Set a new timer interval. We'll do this whenever a new plugin loads, @@ -184,7 +186,7 @@ class MainContext { * set to. */ void update_timer_interval( - std::chrono::steady_clock::duration new_interval); + std::chrono::steady_clock::duration new_interval) noexcept; /** * The RAII guard used to register and unregister host bridge instances from @@ -195,13 +197,13 @@ class MainContext { WatchdogGuard(HostBridge& bridge, std::set& watched_bridges, std::mutex& watched_bridges_mutex); - ~WatchdogGuard(); + ~WatchdogGuard() noexcept; WatchdogGuard(const WatchdogGuard&) = delete; WatchdogGuard& operator=(const WatchdogGuard&) = delete; - WatchdogGuard(WatchdogGuard&& o); - WatchdogGuard& operator=(WatchdogGuard&& o); + WatchdogGuard(WatchdogGuard&& o) noexcept; + WatchdogGuard& operator=(WatchdogGuard&& o) noexcept; private: /**