refactor(downloads): substitute RefCells with RwLocks for thread safety

Author: FranciscoTGouveia

src/cli/common.rs

@@ -1,11 +1,10 @@
 //! Just a dumping ground for cli stuff
 
-use std::cell::RefCell;
 use std::fmt::Display;
 use std::fs;
 use std::io::{BufRead, Write};
 use std::path::{Path, PathBuf};
-use std::sync::{Arc, LazyLock, Mutex};
+use std::sync::{Arc, LazyLock, Mutex, RwLock};
 use std::{cmp, env};
 
 use anyhow::{Context, Result, anyhow};
@@ -123,14 +122,14 @@ pub(crate) fn read_line(process: &Process) -> Result<String> {
 
 pub(super) struct Notifier {
     tracker: Mutex<DownloadTracker>,
-    ram_notice_shown: RefCell<bool>,
+    ram_notice_shown: RwLock<bool>,
 }
 
 impl Notifier {
     pub(super) fn new(quiet: bool, process: &Process) -> Self {
         Self {
             tracker: Mutex::new(DownloadTracker::new_with_display_progress(!quiet, process)),
-            ram_notice_shown: RefCell::new(false),
+            ram_notice_shown: RwLock::new(false),
         }
     }
 
@@ -140,10 +139,10 @@ impl Notifier {
         }
 
         if let Notification::SetDefaultBufferSize(_) = &n {
-            if *self.ram_notice_shown.borrow() {
+            if *self.ram_notice_shown.read().unwrap() {
                 return;
             } else {
-                *self.ram_notice_shown.borrow_mut() = true;
+                *self.ram_notice_shown.write().unwrap() = true;
             }
         };
         let level = n.level();

src/settings.rs

@@ -1,8 +1,8 @@
-use std::cell::RefCell;
 use std::collections::BTreeMap;
 use std::fmt;
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
+use std::sync::RwLock;
 
 use anyhow::{Context, Result};
 use serde::{Deserialize, Serialize};
@@ -13,22 +13,22 @@ use crate::errors::*;
 use crate::notifications::*;
 use crate::utils;
 
-#[derive(Clone, Debug, Eq, PartialEq)]
+#[derive(Debug)]
 pub struct SettingsFile {
     path: PathBuf,
-    cache: RefCell<Option<Settings>>,
+    cache: RwLock<Option<Settings>>,
 }
 
 impl SettingsFile {
     pub(crate) fn new(path: PathBuf) -> Self {
         Self {
             path,
-            cache: RefCell::new(None),
+            cache: RwLock::default(),
         }
     }
 
     fn write_settings(&self) -> Result<()> {
-        let settings = self.cache.borrow();
+        let settings = self.cache.read().unwrap();
         utils::write_file(
             "settings",
             &self.path,
@@ -40,10 +40,10 @@ impl SettingsFile {
     fn read_settings(&self) -> Result<()> {
         let mut needs_save = false;
         {
-            let b = self.cache.borrow();
+            let b = self.cache.read().unwrap();
             if b.is_none() {
                 drop(b);
-                *self.cache.borrow_mut() = Some(if utils::is_file(&self.path) {
+                *self.cache.write().unwrap() = Some(if utils::is_file(&self.path) {
                     let content = utils::read_file("settings", &self.path)?;
                     Settings::parse(&content).with_context(|| RustupError::ParsingFile {
                         name: "settings",
@@ -65,14 +65,17 @@ impl SettingsFile {
         self.read_settings()?;
 
         // Settings can no longer be None so it's OK to unwrap
-        f(self.cache.borrow().as_ref().unwrap())
+        f(self.cache.read().unwrap().as_ref().unwrap())
     }
 
     pub(crate) fn with_mut<T, F: FnOnce(&mut Settings) -> Result<T>>(&self, f: F) -> Result<T> {
         self.read_settings()?;
 
         // Settings can no longer be None so it's OK to unwrap
-        let result = { f(self.cache.borrow_mut().as_mut().unwrap())? };
+        let result = {
+            let mut result = self.cache.write().unwrap();
+            f(result.as_mut().unwrap())?
+        };
         self.write_settings()?;
         Ok(result)
     }