From 37a9f21b2a7c701d28bfbd2b53d28b9afdf33fce Mon Sep 17 00:00:00 2001
From: Devyn Cairns <devyn.cairns@gmail.com>
Date: Tue, 12 Mar 2024 02:50:13 -0700
Subject: [PATCH] Sync with the plugin garbage collector when setting config
 (#12152)

# Description
This causes `PersistentPlugin` to wait for the plugin garbage collector
to actually receive and process its config when setting it in
`set_gc_config`.

The motivation behind doing this is to make setting GC config in scripts
more deterministic. Before this change we couldn't really guarantee that
the GC could see your config before you started doing other things.

There is a slight cost to performance to doing this - we set config
before each plugin call because we don't necessarily know that it
reflects what's in `$env.config`, and now to do that we have to
synchronize with the GC thread.

This was probably the cause of spuriously failing tests as mentioned by
@sholderbach. Hopefully this fixes it. It might be the case that
launching threads on some platforms (or just on a really busy test
runner) sometimes takes a significant amount of time.

# User-Facing Changes
- possibly slightly worse performance for plugin calls
---
 crates/nu-plugin/src/plugin/gc.rs         | 15 +++++++++++++++
 crates/nu-plugin/src/plugin/persistent.rs |  1 +
 2 files changed, 16 insertions(+)

diff --git a/crates/nu-plugin/src/plugin/gc.rs b/crates/nu-plugin/src/plugin/gc.rs
index beb4c8a703..a98fcf23c7 100644
--- a/crates/nu-plugin/src/plugin/gc.rs
+++ b/crates/nu-plugin/src/plugin/gc.rs
@@ -47,6 +47,15 @@ impl PluginGc {
         let _ = self.sender.send(PluginGcMsg::SetConfig(config));
     }
 
+    /// Ensure all GC messages have been processed
+    pub fn flush(&self) {
+        let (tx, rx) = mpsc::channel();
+        let _ = self.sender.send(PluginGcMsg::Flush(tx));
+        // This will block until the channel is dropped, which could be because the send failed, or
+        // because the GC got the message
+        let _ = rx.recv();
+    }
+
     /// Increment the number of locks held by the plugin
     pub fn increment_locks(&self, amount: i64) {
         let _ = self.sender.send(PluginGcMsg::AddLocks(amount));
@@ -83,6 +92,7 @@ impl PluginGc {
 #[derive(Debug)]
 enum PluginGcMsg {
     SetConfig(PluginGcConfig),
+    Flush(mpsc::Sender<()>),
     AddLocks(i64),
     SetDisabled(bool),
     StopTracking,
@@ -123,6 +133,11 @@ impl PluginGcState {
             PluginGcMsg::SetConfig(config) => {
                 self.config = config;
             }
+            PluginGcMsg::Flush(sender) => {
+                // Rather than sending a message, we just drop the channel, which causes the other
+                // side to disconnect equally well
+                drop(sender);
+            }
             PluginGcMsg::AddLocks(amount) => {
                 self.locks += amount;
                 if self.locks < 0 {
diff --git a/crates/nu-plugin/src/plugin/persistent.rs b/crates/nu-plugin/src/plugin/persistent.rs
index 66226575aa..38d91d80a0 100644
--- a/crates/nu-plugin/src/plugin/persistent.rs
+++ b/crates/nu-plugin/src/plugin/persistent.rs
@@ -176,6 +176,7 @@ impl RegisteredPlugin for PersistentPlugin {
             if let Some(running) = running.as_ref() {
                 // If the plugin is already running, propagate the config change to the running GC
                 running.gc.set_config(gc_config.clone());
+                running.gc.flush();
             }
         }
     }