From 565d9259c3fd719bac97f363b9d071faa9b81fc1 Mon Sep 17 00:00:00 2001
From: Robbert van der Helm <mail@robbertvanderhelm.nl>
Date: Tue, 1 Feb 2022 15:31:16 +0100
Subject: [PATCH] Use weak references to the executor

So this cyclic reference can't keep it alive.
---
 src/context.rs       |  4 ++--
 src/context/linux.rs | 28 +++++++++++++++++++++-------
 src/wrapper/vst3.rs  |  2 +-
 3 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/src/context.rs b/src/context.rs
index 9dbbbdd2..93cec608 100644
--- a/src/context.rs
+++ b/src/context.rs
@@ -16,7 +16,7 @@
 
 //! Different contexts the plugin can use to make callbacks to the host in different...contexts.
 
-use std::sync::Arc;
+use std::sync::Weak;
 
 use crate::params::Param;
 
@@ -66,7 +66,7 @@ where
 {
     /// Create and start a new event loop. The thread this is called on will be designated as the
     /// main thread, so this should be called when constructing the wrapper.
-    fn new_and_spawn(executor: Arc<E>) -> Self;
+    fn new_and_spawn(executor: Weak<E>) -> Self;
 
     /// Either post the function to the task queue so it can be delegated to the main thread, or
     /// execute the task directly if this is the main thread. This function needs to be callable at
diff --git a/src/context/linux.rs b/src/context/linux.rs
index 906b475b..cefafb9c 100644
--- a/src/context/linux.rs
+++ b/src/context/linux.rs
@@ -19,7 +19,7 @@
 //! delegate expensive processing to another thread.
 
 use crossbeam::channel;
-use std::sync::Arc;
+use std::sync::Weak;
 use std::thread::{self, JoinHandle, ThreadId};
 
 use super::{EventLoop, MainThreadExecutor};
@@ -31,7 +31,7 @@ pub(crate) struct LinuxEventLoop<T, E> {
     /// The thing that ends up executing these tasks. The tasks are usually executed from the worker
     /// thread, but if the current thread is the main thread then the task cna also be executed
     /// directly.
-    executor: Arc<E>,
+    executor: Weak<E>,
 
     /// The ID of the main thread. In practice this is the ID of the thread that created this task
     /// queue.
@@ -59,7 +59,7 @@ where
     T: Send,
     E: MainThreadExecutor<T>,
 {
-    fn new_and_spawn(executor: Arc<E>) -> Self {
+    fn new_and_spawn(executor: Weak<E>) -> Self {
         let (sender, receiver) = channel::bounded(super::TASK_QUEUE_CAPACITY);
 
         Self {
@@ -83,8 +83,16 @@ where
 
     fn do_maybe_async(&self, task: T) -> bool {
         if self.is_main_thread() {
-            self.executor.execute(task);
-            true
+            match self.executor.upgrade() {
+                Some(e) => {
+                    e.execute(task);
+                    true
+                }
+                None => {
+                    nih_log!("The executor doesn't exist but somehow it's still submitting tasks, this shouldn't be possible!");
+                    false
+                }
+            }
         } else {
             self.worker_thread_channel
                 .try_send(Message::Task(task))
@@ -109,14 +117,20 @@ impl<T, E> Drop for LinuxEventLoop<T, E> {
 }
 
 /// The worker thread used in [EventLoop] that executes incmoing tasks on the event loop's executor.
-fn worker_thread<T, E>(receiver: channel::Receiver<Message<T>>, executor: Arc<E>)
+fn worker_thread<T, E>(receiver: channel::Receiver<Message<T>>, executor: Weak<E>)
 where
     T: Send,
     E: MainThreadExecutor<T>,
 {
     loop {
         match receiver.recv() {
-            Ok(Message::Task(task)) => executor.execute(task),
+            Ok(Message::Task(task)) => match executor.upgrade() {
+                Some(e) => e.execute(task),
+                None => {
+                    nih_log!("Received a new task but the executor is no longer alive, shutting down worker");
+                    return;
+                }
+            },
             Ok(Message::Shutdown) => return,
             Err(err) => {
                 nih_log!(
diff --git a/src/wrapper/vst3.rs b/src/wrapper/vst3.rs
index a7cec81f..25572a39 100644
--- a/src/wrapper/vst3.rs
+++ b/src/wrapper/vst3.rs
@@ -220,7 +220,7 @@ impl<P: Plugin> WrapperInner<'_, P> {
         // XXX: This unsafe block is unnecessary. rust-analyzer gets a bit confused and this this
         //      `write()` function is from `IBStream` which it definitely is not.
         *unsafe { wrapper.event_loop.write() } =
-            MaybeUninit::new(OsEventLoop::new_and_spawn(wrapper.clone()));
+            MaybeUninit::new(OsEventLoop::new_and_spawn(Arc::downgrade(&wrapper)));
 
         wrapper
     }