Merge pull request #3371 from rbtcollins/threadsafe-download-tracker

Author: 0xPoe

src/cli/common.rs

@@ -1,5 +1,6 @@
 //! Just a dumping ground for cli stuff
 
+use std::cell::RefCell;
 use std::fmt::Display;
 use std::fs;
 use std::io::{BufRead, ErrorKind, Write};
@@ -12,6 +13,7 @@ use git_testament::{git_testament, render_testament};
 use lazy_static::lazy_static;
 
 use super::self_update;
+use crate::cli::download_tracker::DownloadTracker;
 use crate::currentprocess::{
     argsource::ArgSource,
     filesource::{StdinSource, StdoutSource},
@@ -168,18 +170,14 @@ impl NotifyOnConsole {
 
 #[cfg_attr(feature = "otel", tracing::instrument)]
 pub(crate) fn set_globals(verbose: bool, quiet: bool) -> Result<Cfg> {
-    use std::cell::RefCell;
-
-    use super::download_tracker::DownloadTracker;
-
-    let download_tracker = RefCell::new(DownloadTracker::new().with_display_progress(!quiet));
+    let download_tracker = DownloadTracker::new_with_display_progress(!quiet);
     let console_notifier = RefCell::new(NotifyOnConsole {
         verbose,
         ..Default::default()
     });
 
     Cfg::from_env(Arc::new(move |n: Notification<'_>| {
-        if download_tracker.borrow_mut().handle_notification(&n) {
+        if download_tracker.lock().unwrap().handle_notification(&n) {
             return;
         }
         console_notifier.borrow_mut().handle(n);

src/cli/download_tracker.rs

@@ -1,6 +1,7 @@
 use std::collections::VecDeque;
 use std::fmt;
 use std::io::Write;
+use std::sync::{Arc, Mutex};
 use std::time::{Duration, Instant};
 
 use crate::currentprocess::{filesource::StdoutSource, process, terminalsource};
@@ -13,6 +14,9 @@ use crate::Notification;
 const DOWNLOAD_TRACK_COUNT: usize = 5;
 
 /// Tracks download progress and displays information about it to a terminal.
+///
+/// *not* safe for tracking concurrent downloads yet - it is basically undefined
+/// what will happen.
 pub(crate) struct DownloadTracker {
     /// Content-Length of the to-be downloaded object.
     content_len: Option<usize>,
@@ -46,8 +50,8 @@ pub(crate) struct DownloadTracker {
 
 impl DownloadTracker {
     /// Creates a new DownloadTracker.
-    pub(crate) fn new() -> Self {
-        Self {
+    pub(crate) fn new_with_display_progress(display_progress: bool) -> Arc<Mutex<Self>> {
+        Arc::new(Mutex::new(Self {
             content_len: None,
             total_downloaded: 0,
             downloaded_this_sec: 0,
@@ -57,13 +61,8 @@ impl DownloadTracker {
             term: process().stdout().terminal(),
             displayed_charcount: None,
             units: vec![Unit::B],
-            display_progress: true,
-        }
-    }
-
-    pub(crate) fn with_display_progress(mut self, display_progress: bool) -> Self {
-        self.display_progress = display_progress;
-        self
+            display_progress: display_progress,
+        }))
     }
 
     pub(crate) fn handle_notification(&mut self, n: &Notification<'_>) -> bool {

src/cli/self_update/windows.rs

@@ -1,4 +1,3 @@
-use std::cell::RefCell;
 use std::env::{consts::EXE_SUFFIX, split_paths};
 use std::ffi::{OsStr, OsString};
 use std::fmt;
@@ -181,13 +180,14 @@ pub(crate) fn try_install_msvc(opts: &InstallOpts<'_>) -> Result<ContinueInstall
         .context("error creating temp directory")?;
 
     let visual_studio = tempdir.path().join("vs_setup.exe");
-    let download_tracker = RefCell::new(DownloadTracker::new().with_display_progress(true));
-    download_tracker.borrow_mut().download_finished();
+    let download_tracker = DownloadTracker::new_with_display_progress(true);
+    download_tracker.lock().unwrap().download_finished();
 
     info!("downloading Visual Studio installer");
     utils::download_file(&visual_studio_url, &visual_studio, None, &move |n| {
         download_tracker
-            .borrow_mut()
+            .lock()
+            .unwrap()
             .handle_notification(&crate::Notification::Install(
                 crate::dist::Notification::Utils(n),
             ));