Clean up Win32 thread API usage using RAII

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
     }
 }
 

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

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 <https://www.gnu.org/licenses/>.
+
+#include "utils.h"
+
+Win32Thread::Win32Thread() : handle(nullptr, nullptr) {}

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 <https://www.gnu.org/licenses/>.
+
+#include <memory>
+
+#define NOMINMAX
+#define NOSERVICE
+#define NOMCX
+#define NOIMM
+#define WIN32_LEAN_AND_MEAN
+#include <windows.h>
+
+/**
+ * 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 <typename F>
+    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<std::remove_pointer_t<HANDLE>, decltype(&CloseHandle)>
+        handle;
+};