diff --git a/CHANGELOG.md b/CHANGELOG.md index 55695d37..e2ba86ce 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/cross-wine.conf b/cross-wine.conf index 1033d8d7..0d7ff22b 100644 --- a/cross-wine.conf +++ b/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'] diff --git a/src/common/communication/common.h b/src/common/communication/common.h index e430864c..e45f06a3 100644 --- a/src/common/communication/common.h +++ b/src/common/communication/common.h @@ -107,7 +107,8 @@ template inline T& read_object(Socket& socket, std::vector& buffer, T& object) { // See the note above on the use of `uint64_t` instead of `size_t` std::array 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& 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)); - assert(size == actual_size); + boost::asio::read(socket, boost::asio::buffer(buffer), + boost::asio::transfer_exactly(size)); auto [_, success] = bitsery::quickDeserialization>>( @@ -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 - // process does an `audioMaster()` call before the plugin is - // listening. If that happens we'll fall back to a synchronous - // request. This is not very pretty, so if anyone can think of a - // better way to structure all of this while still mainting a - // long living primary socket please let me know. - std::lock_guard lock(write_mutex); + // yet? This can happen with plugin groups when the Wine + // host process does an `audioMaster()` call before the + // plugin is listening. If that happens we'll fall back to a + // synchronous request. This is not very pretty, so if + // anyone can think of a better way to structure all of this + // while still mainting a long living primary socket please + // 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; }; diff --git a/src/wine-host/bridges/group.cpp b/src/wine-host/bridges/group.cpp index d38cbf49..579988c1 100644 --- a/src/wine-host/bridges/group.cpp +++ b/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()) { diff --git a/src/wine-host/bridges/group.h b/src/wine-host/bridges/group.h index 7c0e65a3..c45bd494 100644 --- a/src/wine-host/bridges/group.h +++ b/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; };