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
This commit is contained in:
Devyn Cairns 2024-03-12 02:50:13 -07:00 committed by GitHub
parent 73f3c0b60b
commit 37a9f21b2a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 16 additions and 0 deletions

View File

@ -47,6 +47,15 @@ impl PluginGc {
let _ = self.sender.send(PluginGcMsg::SetConfig(config)); 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 /// Increment the number of locks held by the plugin
pub fn increment_locks(&self, amount: i64) { pub fn increment_locks(&self, amount: i64) {
let _ = self.sender.send(PluginGcMsg::AddLocks(amount)); let _ = self.sender.send(PluginGcMsg::AddLocks(amount));
@ -83,6 +92,7 @@ impl PluginGc {
#[derive(Debug)] #[derive(Debug)]
enum PluginGcMsg { enum PluginGcMsg {
SetConfig(PluginGcConfig), SetConfig(PluginGcConfig),
Flush(mpsc::Sender<()>),
AddLocks(i64), AddLocks(i64),
SetDisabled(bool), SetDisabled(bool),
StopTracking, StopTracking,
@ -123,6 +133,11 @@ impl PluginGcState {
PluginGcMsg::SetConfig(config) => { PluginGcMsg::SetConfig(config) => {
self.config = 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) => { PluginGcMsg::AddLocks(amount) => {
self.locks += amount; self.locks += amount;
if self.locks < 0 { if self.locks < 0 {

View File

@ -176,6 +176,7 @@ impl RegisteredPlugin for PersistentPlugin {
if let Some(running) = running.as_ref() { if let Some(running) = running.as_ref() {
// If the plugin is already running, propagate the config change to the running GC // If the plugin is already running, propagate the config change to the running GC
running.gc.set_config(gc_config.clone()); running.gc.set_config(gc_config.clone());
running.gc.flush();
} }
} }
} }