fix: prevent deadlock when resetting

jdx · Nov 29, 2024 · 169338a · 169338a
1 parent 201ba90
commit 169338a
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 27 deletions.
diff --git 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
@@ -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<String, ABackend>;
 pub type BackendList = Vec<ABackend>;
 pub type VersionCacheManager = CacheManager<Vec<String>>;
 
-static TOOLS: Mutex<Option<BackendMap>> = Mutex::new(None);
+static TOOLS: Mutex<Option<Arc<BackendMap>>> = Mutex::new(None);
 
-fn load_tools() -> MutexGuard<'static, Option<BackendMap>> {
+fn load_tools() -> MutexGuard<'static, Option<Arc<BackendMap>>> {
     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<BackendMap>> {
                 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<BackendMap>> {
         .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<ABackend> {
     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<ABackend> {
 
 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<ABackend> {

diff --git 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
@@ -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::<Vec<_>>(),
         };
 

diff --git a/src/plugins/mod.rs b/src/plugins/mod.rs
@@ -54,8 +54,8 @@ pub static VERSION_REGEX: Lazy<regex::Regex> = Lazy::new(|| {
 
 pub fn get(short: &str) -> Result<APlugin> {
     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
@@ -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<String, PluginType>;
 type InstallStateTools = BTreeMap<String, InstallStateTool>;
+type MutexResult<T> = Result<MutexGuard<'static, Option<Arc<T>>>>;
 
 #[derive(Debug, Clone)]
 pub struct InstallStateTool {
@@ -22,8 +24,8 @@ pub struct InstallStateTool {
     pub versions: Vec<String>,
 }
 
-static INSTALL_STATE_PLUGINS: Mutex<Option<InstallStatePlugins>> = Mutex::new(None);
-static INSTALL_STATE_TOOLS: Mutex<Option<InstallStateTools>> = Mutex::new(None);
+static INSTALL_STATE_PLUGINS: Mutex<Option<Arc<InstallStatePlugins>>> = Mutex::new(None);
+static INSTALL_STATE_TOOLS: Mutex<Option<Arc<InstallStateTools>>> = 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<MutexGuard<'static, Option<BTreeMap<String, PluginType>>>> {
+fn init_plugins() -> MutexResult<InstallStatePlugins> {
     let mut mu = INSTALL_STATE_PLUGINS.lock().unwrap();
     if mu.is_some() {
         return Ok(mu);
@@ -63,11 +65,11 @@ fn init_plugins() -> Result<MutexGuard<'static, Option<BTreeMap<String, PluginTy
             }
         })
         .collect();
-    *mu = Some(plugins);
+    *mu = Some(Arc::new(plugins));
     Ok(mu)
 }
 
-fn init_tools() -> Result<MutexGuard<'static, Option<BTreeMap<String, InstallStateTool>>>> {
+fn init_tools() -> MutexResult<InstallStateTools> {
     let mut mu = INSTALL_STATE_TOOLS.lock().unwrap();
     if mu.is_some() {
         return Ok(mu);
@@ -103,7 +105,7 @@ fn init_tools() -> Result<MutexGuard<'static, Option<BTreeMap<String, InstallSta
         .flatten()
         .filter(|(_, tool)| !tool.versions.is_empty())
         .collect::<BTreeMap<_, _>>();
-    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<MutexGuard<'static, Option<BTreeMap<String, InstallSta
             });
         tool.full = Some(full);
     }
-    *mu = Some(tools);
+    *mu = Some(Arc::new(tools));
     Ok(mu)
 }
 
-pub fn list_plugins() -> Result<BTreeMap<String, PluginType>> {
+pub fn list_plugins() -> Result<Arc<BTreeMap<String, PluginType>>> {
     let plugins = init_plugins()?;
     Ok(plugins.as_ref().unwrap().clone())
 }
@@ -140,7 +142,7 @@ pub fn get_plugin_type(short: &str) -> Result<Option<PluginType>> {
     Ok(plugins.as_ref().unwrap().get(short).cloned())
 }
 
-pub fn list_tools() -> Result<BTreeMap<String, InstallStateTool>> {
+pub fn list_tools() -> Result<Arc<BTreeMap<String, InstallStateTool>>> {
     let tools = init_tools()?;
     Ok(tools.as_ref().unwrap().clone())
 }
@@ -167,11 +169,10 @@ pub fn list_versions(short: &str) -> Result<Vec<String>> {
 }
 
 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(())
 }