From 169338a2debb99ee4dd885376c4123740237af23 Mon Sep 17 00:00:00 2001 From: jdx <216188+jdx@users.noreply.github.com> Date: Thu, 28 Nov 2024 22:17:02 -0600 Subject: [PATCH] fix: prevent deadlock when resetting --- e2e/run_test | 1 + src/backend/mod.rs | 16 ++++++++++------ src/cli/plugins/ls.rs | 4 ++-- src/cli/plugins/update.rs | 4 ++-- src/plugins/mod.rs | 4 ++-- src/toolset/install_state.rs | 31 ++++++++++++++++--------------- 6 files changed, 33 insertions(+), 27 deletions(-) diff --git a/e2e/run_test b/e2e/run_test index 3fa5644a18..a8db99d6e2 100755 --- a/e2e/run_test +++ b/e2e/run_test @@ -55,6 +55,7 @@ within_isolated_env() { CARGO_LLVM_COV_SHOW_ENV="${CARGO_LLVM_COV_SHOW_ENV:-}" \ CARGO_LLVM_COV_TARGET_DIR="${CARGO_LLVM_COV_TARGET_DIR:-}" \ GITHUB_ACTION="${GITHUB_ACTION:-}" \ + GITHUB_TOKEN="${GITHUB_TOKEN:-}" \ GITHUB_API_TOKEN="${GITHUB_API_TOKEN:-}" \ GOPROXY="${GOPROXY:-}" \ HOME="$TEST_HOME" \ diff --git a/src/backend/mod.rs b/src/backend/mod.rs index 45124871d5..97fc0709ff 100644 --- a/src/backend/mod.rs +++ b/src/backend/mod.rs @@ -2,6 +2,7 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::fmt::{Debug, Display, Formatter}; use std::fs::File; use std::hash::Hash; +use std::ops::Deref; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex, MutexGuard}; @@ -45,9 +46,9 @@ pub type BackendMap = BTreeMap; pub type BackendList = Vec; pub type VersionCacheManager = CacheManager>; -static TOOLS: Mutex> = Mutex::new(None); +static TOOLS: Mutex>> = Mutex::new(None); -fn load_tools() -> MutexGuard<'static, Option> { +fn load_tools() -> MutexGuard<'static, Option>> { let mut memo_tools = TOOLS.lock().unwrap(); if memo_tools.is_some() { return memo_tools; @@ -62,9 +63,9 @@ fn load_tools() -> MutexGuard<'static, Option> { warn!("{err:#}"); }) .unwrap_or_default() - .into_values() + .values() .filter(|ist| ist.full.is_some()) - .flat_map(|ist| arg_to_backend(ist.into())), + .flat_map(|ist| arg_to_backend(ist.clone().into())), ); time!("load_tools install_state"); tools.retain(|backend| !SETTINGS.disable_tools().contains(backend.id())); @@ -78,7 +79,7 @@ fn load_tools() -> MutexGuard<'static, Option> { .into_iter() .map(|backend| (backend.ba().short.clone(), backend)) .collect(); - *memo_tools = Some(tools.clone()); + *memo_tools = Some(Arc::new(tools.clone())); time!("load_tools done"); memo_tools } @@ -93,7 +94,9 @@ pub fn get(ba: &BackendArg) -> Option { if let Some(backend) = backends.get(&ba.short) { Some(backend.clone()) } else if let Some(backend) = arg_to_backend(ba.clone()) { + let mut backends = (**backends).clone(); backends.insert(ba.short.clone(), backend.clone()); + *m = Some(Arc::new(backends)); Some(backend) } else { None @@ -102,8 +105,9 @@ pub fn get(ba: &BackendArg) -> Option { pub fn remove(short: &str) { let mut t = load_tools(); - let backends = t.as_mut().unwrap(); + let mut backends = t.clone().unwrap().deref().clone(); backends.remove(short); + *t = Some(Arc::new(backends)); } pub fn arg_to_backend(ba: BackendArg) -> Option { diff --git a/src/cli/plugins/ls.rs b/src/cli/plugins/ls.rs index e4b22ca87e..7056cd4934 100644 --- a/src/cli/plugins/ls.rs +++ b/src/cli/plugins/ls.rs @@ -44,8 +44,8 @@ pub struct PluginsLs { impl PluginsLs { pub fn run(self, config: &Config) -> Result<()> { let mut plugins: BTreeMap<_, _> = install_state::list_plugins()? - .into_iter() - .map(|(k, p)| (k, (p, None))) + .iter() + .map(|(k, p)| (k.clone(), (*p, None))) .collect(); if self.core { diff --git a/src/cli/plugins/update.rs b/src/cli/plugins/update.rs index 04649e8f3c..40027b2b60 100644 --- a/src/cli/plugins/update.rs +++ b/src/cli/plugins/update.rs @@ -34,8 +34,8 @@ impl Update { }) .collect(), None => install_state::list_plugins()? - .into_keys() - .map(|p| (p, None)) + .keys() + .map(|p| (p.clone(), None)) .collect::>(), }; diff --git a/src/plugins/mod.rs b/src/plugins/mod.rs index 89b104b308..95f3959cde 100644 --- a/src/plugins/mod.rs +++ b/src/plugins/mod.rs @@ -54,8 +54,8 @@ pub static VERSION_REGEX: Lazy = Lazy::new(|| { pub fn get(short: &str) -> Result { let (name, full) = short.split_once(':').unwrap_or((short, short)); - let plugin_type = if let Some(plugin_type) = install_state::list_plugins()?.remove(short) { - plugin_type + let plugin_type = if let Some(plugin_type) = install_state::list_plugins()?.get(short) { + *plugin_type } else { PluginType::from_full(full)? }; diff --git a/src/toolset/install_state.rs b/src/toolset/install_state.rs index 505f046a45..881d698aa6 100644 --- a/src/toolset/install_state.rs +++ b/src/toolset/install_state.rs @@ -8,12 +8,14 @@ use heck::ToKebabCase; use itertools::Itertools; use rayon::prelude::*; use std::collections::BTreeMap; +use std::ops::Deref; use std::path::PathBuf; -use std::sync::{Mutex, MutexGuard}; +use std::sync::{Arc, Mutex, MutexGuard}; use versions::Versioning; type InstallStatePlugins = BTreeMap; type InstallStateTools = BTreeMap; +type MutexResult = Result>>>; #[derive(Debug, Clone)] pub struct InstallStateTool { @@ -22,8 +24,8 @@ pub struct InstallStateTool { pub versions: Vec, } -static INSTALL_STATE_PLUGINS: Mutex> = Mutex::new(None); -static INSTALL_STATE_TOOLS: Mutex> = Mutex::new(None); +static INSTALL_STATE_PLUGINS: Mutex>> = Mutex::new(None); +static INSTALL_STATE_TOOLS: Mutex>> = Mutex::new(None); pub(crate) fn init() -> Result<()> { let (plugins, tools) = rayon::join( @@ -43,7 +45,7 @@ pub(crate) fn init() -> Result<()> { Ok(()) } -fn init_plugins() -> Result>>> { +fn init_plugins() -> MutexResult { let mut mu = INSTALL_STATE_PLUGINS.lock().unwrap(); if mu.is_some() { return Ok(mu); @@ -63,11 +65,11 @@ fn init_plugins() -> Result Result>>> { +fn init_tools() -> MutexResult { let mut mu = INSTALL_STATE_TOOLS.lock().unwrap(); if mu.is_some() { return Ok(mu); @@ -103,7 +105,7 @@ fn init_tools() -> Result>(); - for (short, pt) in init_plugins()?.as_ref().unwrap() { + for (short, pt) in init_plugins()?.as_ref().unwrap().iter() { let full = match pt { PluginType::Asdf => format!("asdf:{short}"), PluginType::Vfox => format!("vfox:{short}"), @@ -117,11 +119,11 @@ fn init_tools() -> Result Result> { +pub fn list_plugins() -> Result>> { let plugins = init_plugins()?; Ok(plugins.as_ref().unwrap().clone()) } @@ -140,7 +142,7 @@ pub fn get_plugin_type(short: &str) -> Result> { Ok(plugins.as_ref().unwrap().get(short).cloned()) } -pub fn list_tools() -> Result> { +pub fn list_tools() -> Result>> { let tools = init_tools()?; Ok(tools.as_ref().unwrap().clone()) } @@ -167,11 +169,10 @@ pub fn list_versions(short: &str) -> Result> { } pub fn add_plugin(short: &str, plugin_type: PluginType) -> Result<()> { - let mut plugins = init_plugins()?; - plugins - .as_mut() - .unwrap() - .insert(short.to_string(), plugin_type); + let mut p = init_plugins()?; + let mut plugins = p.take().unwrap().deref().clone(); + plugins.insert(short.to_string(), plugin_type); + *p = Some(Arc::new(plugins)); Ok(()) }