Merge branch 'master' into feature/vst3

CHANGELOG.md

@@ -30,6 +30,16 @@ Versioning](https://semver.org/spec/v2.0.0.html).
 - Symbols in all `libyabridge.so` and all Winelib `.so` files are now hidden by
   default.
 
+### Fixed
+
+- Fixed an issue where in certain situations Wine processes were left running
+  after the host got forcefully terminated before it got a chance to tell the
+  plugin to shut down. This could happen when using Kontakt in Bitwig, as Bitwig
+  sets a limit on the amount of time a plugin may take to shut down when closing
+  Bitwig.
+- Fixed a potential crash or freeze when removing a lot of plugins from a plugin
+  group at exactly the same time.
+
 ## [2.1.0] - 2020-11-20
 
 ### Added

cross-wine.conf

@@ -17,9 +17,9 @@ needs_exe_wrapper = true
 # applications are always in sync. This might not be needed anymore once Meson
 # cross compiling to multiple targets at once.
 # https://github.com/mesonbuild/meson/issues/5125
-cpp_link_args = ['-mwindows']
+cpp_link_args = ['-mconsole']
 
 # For instance, this should be in the 64-bit only cross-file
 # c_args = ['-m64']
 # cpp_args = ['-m64']
-# cpp_link_args = ['-m64', '-mwindows']
+# cpp_link_args = ['-m64', '-mconsole']

src/common/communication/common.h

@@ -107,7 +107,8 @@ template <typename T, typename Socket>
 inline T& read_object(Socket& socket, std::vector<uint8_t>& buffer, T& object) {
     // See the note above on the use of `uint64_t` instead of `size_t`
     std::array<uint64_t, 1> message_length;
-    boost::asio::read(socket, boost::asio::buffer(message_length));
+    boost::asio::read(socket, boost::asio::buffer(message_length),
+                      boost::asio::transfer_exactly(sizeof(message_length)));
 
     // Make sure the buffer is large enough
     const size_t size = message_length[0];
@@ -116,9 +117,8 @@ inline T& read_object(Socket& socket, std::vector<uint8_t>& buffer, T& object) {
     // `boost::asio::read/write` will handle all the packet splitting and
     // merging for us, since local domain sockets have packet limits somewhere
     // in the hundreds of kilobytes
-    const auto actual_size =
+    boost::asio::read(socket, boost::asio::buffer(buffer),
-        boost::asio::read(socket, boost::asio::buffer(buffer));
+                      boost::asio::transfer_exactly(size));
-    assert(size == actual_size);
 
     auto [_, success] =
         bitsery::quickDeserialization<InputAdapter<std::vector<uint8_t>>>(
@@ -536,7 +536,13 @@ class AdHocSocketHandler {
         //      we might be able to do some optimizations there.
         std::unique_lock lock(write_mutex, std::try_to_lock);
         if (lock.owns_lock()) {
-            return callback(socket);
+            // This was used to always block when sending the first message,
+            // because the other side may not be listening for additional
+            // connections yet
+            auto result = callback(socket);
+            sent_first_event = true;
+
+            return result;
         } else {
             try {
                 boost::asio::local::stream_protocol::socket secondary_socket(
@@ -544,17 +550,31 @@ class AdHocSocketHandler {
                 secondary_socket.connect(endpoint);
 
                 return callback(secondary_socket);
-            } catch (const boost::system::system_error&) {
+            } catch (const boost::system::system_error& e) {
                 // So, what do we do when noone is listening on the endpoint
-                // yet? This can happen with plugin groups when the Wine host
+                // yet? This can happen with plugin groups when the Wine
-                // process does an `audioMaster()` call before the plugin is
+                // host process does an `audioMaster()` call before the
-                // listening. If that happens we'll fall back to a synchronous
+                // plugin is listening. If that happens we'll fall back to a
-                // request. This is not very pretty, so if anyone can think of a
+                // synchronous request. This is not very pretty, so if
-                // better way to structure all of this while still mainting a
+                // anyone can think of a better way to structure all of this
-                // long living primary socket please let me know.
+                // while still mainting a long living primary socket please
-                std::lock_guard lock(write_mutex);
+                // let me know.
+                // Note that this should **only** be done before the call to
+                // `connect()`. If we get here at any other point then it
+                // means that the plugin side is no longer listening on the
+                // sockets, and we should thus just exit.
+                if (!sent_first_event) {
+                    std::lock_guard lock(write_mutex);
 
-                return callback(socket);
+                    auto result = callback(socket);
+                    sent_first_event = true;
+
+                    return result;
+                } else {
+                    // Rethrow the exception if the sockets we're not
+                    // handling the specific case described above
+                    throw e;
+                }
             }
         }
     }
@@ -732,4 +752,13 @@ class AdHocSocketHandler {
      * events will be sent over a new socket instead.
      */
     std::mutex write_mutex;
+
+    /**
+     * Indicates whether or not the remove has processed an event we sent from
+     * this side. When a Windows VST2 plugin performs a host callback in its
+     * constructor, before the native plugin has had time to connect to the
+     * sockets, we want it to always wait for the sockets to come online, but
+     * this fallback behaviour should only happen during initialization.
+     */
+    std::atomic_bool sent_first_event = false;
 };

src/wine-host/bridges/group.cpp

@@ -141,8 +141,9 @@ void GroupBridge::handle_plugin_dispatch(size_t plugin_id) {
 
     // Defer actually shutting down the process to allow for fast plugin
     // scanning by allowing plugins to reuse the same group host process
+    std::lock_guard lock(shutdown_timer_mutex);
     shutdown_timer.expires_after(2s);
-    shutdown_timer.async_wait([&](const boost::system::error_code& error) {
+    shutdown_timer.async_wait([this](const boost::system::error_code& error) {
         // A previous timer gets canceled automatically when another plugin
         // exits
         if (error.failed()) {

src/wine-host/bridges/group.h

@@ -278,4 +278,9 @@ class GroupBridge {
      * @see handle_plugin_dispatch
      */
     boost::asio::steady_timer shutdown_timer;
+    /**
+     * A mutex to prevent two threads from simultaneously modifying the shutdown
+     * timer when multiple plugins exit at the same time.
+     */
+    std::mutex shutdown_timer_mutex;
 };