From ead4ca97c54a5db68732f0fca76521ac0c16ad7b Mon Sep 17 00:00:00 2001
From: Robbert van der Helm <mail@robbertvanderhelm.nl>
Date: Thu, 20 Apr 2023 15:38:28 +0200
Subject: [PATCH] Fix race condition in CLAP request callback impl

This would deadlock if the host simultaneously tries to create a plugin
instance.
---
 CHANGELOG.md                                  |  4 ++++
 .../bridges/clap-impls/host-proxy.cpp         | 23 +++++++++++--------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 80929c3e..d6cf0066 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -24,6 +24,10 @@ Versioning](https://semver.org/spec/v2.0.0.html).
   tried to query a parameter value with signed index -1. This has now been
   fixed. The issue only appeared with the VST3 validator, and not with any
   regular hosts.
+- Fixed a race condition that could occur when a CLAP plugin instance would
+  request a host callback while the host simultaneously tried to create another
+  instance of the same plugin. This would result in a deadlock. An example of a
+  plugin that triggered this is _PolyChrome DSP's McRocklin Suite_.
 
 ### yabridgectl
 
diff --git a/src/wine-host/bridges/clap-impls/host-proxy.cpp b/src/wine-host/bridges/clap-impls/host-proxy.cpp
index a6522f17..23fd874b 100644
--- a/src/wine-host/bridges/clap-impls/host-proxy.cpp
+++ b/src/wine-host/bridges/clap-impls/host-proxy.cpp
@@ -188,21 +188,26 @@ clap_host_proxy::host_request_callback(const struct clap_host* host) {
     bool expected = false;
     if (self->has_pending_host_callbacks_.compare_exchange_strong(expected,
                                                                   true)) {
-        // We're acquiring a lock on the instance and then move it into the task
-        // to prevent this instance from being removed before this callback has
-        // been run
-        auto instance_lock =
-            self->bridge_.get_instance(self->owner_instance_id());
-        self->bridge_.main_context_.schedule_task(
-            [self, instance_lock = std::move(instance_lock)]() {
-                const auto& [instance, _] = instance_lock;
+        self->bridge_.main_context_.schedule_task([self]() {
+            try {
+                const auto& [instance, _] =
+                    self->bridge_.get_instance(self->owner_instance_id());
                 self->has_pending_host_callbacks_.store(false);
 
                 self->bridge_.logger_.log_on_main_thread(
                     self->owner_instance_id());
 
                 instance.plugin->on_main_thread(instance.plugin.get());
-            });
+            } catch (const std::out_of_range&) {
+                // In case the plugin has been removed in the meantime, we
+                // should just ignore that silently. A previous version of this
+                // acquired the lock before the `schedule_task()` call and moved
+                // it into this closure, but that leads to a nasty difficult to
+                // track down race condition when a plugin requests a callback
+                // while at the same time the host tries to create another
+                // plugin instance (which requires unique access to the mutex).
+            }
+        });
     }
 }