From d0c8d8a2e7a635eab70f5c9e02da1180157e06e7 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Fri, 24 Apr 2020 15:39:43 +0200 Subject: [PATCH] Clean up Win32 thread API usage using RAII --- meson.build | 4 +- src/wine-host/plugin-bridge.cpp | 18 ++++---- src/wine-host/plugin-bridge.h | 12 ++---- src/wine-host/utils.cpp | 19 +++++++++ src/wine-host/utils.h | 73 +++++++++++++++++++++++++++++++++ 5 files changed, 106 insertions(+), 20 deletions(-) create mode 100644 src/wine-host/utils.cpp create mode 100644 src/wine-host/utils.h diff --git a/meson.build b/meson.build index f2a2f5f0..65144f82 100644 --- a/meson.build +++ b/meson.build @@ -56,9 +56,11 @@ executable( [ 'src/common/logging.cpp', 'src/common/serialization.cpp', + 'src/wine-host/editor.cpp', + 'src/wine-host/editor.cpp', 'src/wine-host/plugin-bridge.cpp', 'src/wine-host/vst-host.cpp', - 'src/wine-host/editor.cpp', + 'src/wine-host/utils.cpp', ], native : false, include_directories : include_dir, diff --git a/src/wine-host/plugin-bridge.cpp b/src/wine-host/plugin-bridge.cpp index 533b6b81..a5e3e0a1 100644 --- a/src/wine-host/plugin-bridge.cpp +++ b/src/wine-host/plugin-bridge.cpp @@ -129,14 +129,13 @@ PluginBridge::PluginBridge(std::string plugin_dll_path, // This works functionally identically to the `handle_dispatch()` function // below, but this socket will only handle midi events. This is needed // because of Win32 API limitations. - dispatch_midi_events_handler = CreateThread( - nullptr, 0, handle_dispatch_midi_events_proxy, this, 0, nullptr); + dispatch_midi_events_handler = + Win32Thread(handle_dispatch_midi_events_proxy, this); - parameters_handler = - CreateThread(nullptr, 0, handle_parameters_proxy, this, 0, nullptr); + parameters_handler = Win32Thread(handle_parameters_proxy, this); - process_replacing_handler = CreateThread( - nullptr, 0, handle_process_replacing_proxy, this, 0, nullptr); + process_replacing_handler = + Win32Thread(handle_process_replacing_proxy, this); std::cout << "Finished initializing '" << plugin_dll_path << "'" << std::endl; @@ -155,11 +154,8 @@ void PluginBridge::handle_dispatch() { _1, _2, _3, _4, _5, _6)); } } catch (const boost::system::system_error&) { - // TODO: Implement this with a simple RAII wrapper just like we did for - // Win32 window classes - CloseHandle(dispatch_midi_events_handler); - CloseHandle(parameters_handler); - CloseHandle(process_replacing_handler); + // The plugin has cut off communications, so we can shut down this host + // application } } diff --git a/src/wine-host/plugin-bridge.h b/src/wine-host/plugin-bridge.h index 72b93c33..bf2cc78f 100644 --- a/src/wine-host/plugin-bridge.h +++ b/src/wine-host/plugin-bridge.h @@ -31,6 +31,7 @@ #include "../common/logging.h" #include "editor.h" +#include "utils.h" /** * This handles the communication between the Linux native VST plugin and the @@ -139,25 +140,20 @@ class PluginBridge { */ boost::asio::local::stream_protocol::socket vst_host_aeffect; - // These threads are implemented using `CreateThread` rather than - // `std::thread` because in some cases `std::thread` in winelib causes very - // hard to debug data races within plugins such as Serum. This might be - // caused by calling conventions being handled differently. - /** * The thread that specifically handles `effProcessEvents` opcodes so the * plugin can still receive midi during GUI interaction to work around Win32 * API limitations. */ - HANDLE dispatch_midi_events_handler; + Win32Thread dispatch_midi_events_handler; /** * The thread that responds to `getParameter` and `setParameter` requests. */ - HANDLE parameters_handler; + Win32Thread parameters_handler; /** * The thread that handles calls to `processReplacing` (and `process`). */ - HANDLE process_replacing_handler; + Win32Thread process_replacing_handler; /** * A binary semaphore to prevent race conditions from the host callback diff --git a/src/wine-host/utils.cpp b/src/wine-host/utils.cpp new file mode 100644 index 00000000..0c9a5d62 --- /dev/null +++ b/src/wine-host/utils.cpp @@ -0,0 +1,19 @@ +// yabridge: a Wine VST bridge +// Copyright (C) 2020 Robbert van der Helm +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +#include "utils.h" + +Win32Thread::Win32Thread() : handle(nullptr, nullptr) {} diff --git a/src/wine-host/utils.h b/src/wine-host/utils.h new file mode 100644 index 00000000..32c7e51b --- /dev/null +++ b/src/wine-host/utils.h @@ -0,0 +1,73 @@ +// yabridge: a Wine VST bridge +// Copyright (C) 2020 Robbert van der Helm +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +#include + +#define NOMINMAX +#define NOSERVICE +#define NOMCX +#define NOIMM +#define WIN32_LEAN_AND_MEAN +#include + +/** + * A simple RAII wrapper around the Win32 thread API. + * + * These threads are implemented using `CreateThread` rather than `std::thread` + * because in some cases `std::thread` in winelib causes very hard to debug data + * races within plugins such as Serum. This might be caused by calling + * conventions being handled differently. + * + * This somewhat mimicks `std::thread`, with the following differences: + * + * - The threads will immediatly be killed silently when a `Win32Thread` object + * goes out of scope. This is the desired behavior in our case since the host + * will have already saved chunk data before closing the plugin and this + * ensures that the plugin shuts down quickly. + * - This does not accept lambdas because we're calling a C function that + * expects a function pointer of type `LPTHREAD_START_ROUTINE`. GCC supports + * converting stateless lambdas to this format, but clang (as used for IDE + * tooling) does not. + */ +class Win32Thread { + public: + /** + * Constructor that does not start any thread yet. + */ + Win32Thread(); + + /** + * Constructor that immediately starts running the thread + * + * @param entry_point The thread entry point that should be run. + * @param parameter The parameter passed to the entry point function. + + * @tparam F A function type that should be convertible to a + * `LPTHREAD_START_ROUTINE` function pointer. + */ + template + Win32Thread(F entry_point, void* parameter) + : handle(CreateThread(nullptr, 0, entry_point, parameter, 0, nullptr), + CloseHandle) {} + + private: + /** + * The handle for the thread that is running, will be a null pointer if this + * class was constructed with the default constructor. + */ + std::unique_ptr, decltype(&CloseHandle)> + handle; +};