updated progress reporter to use refcell and atomics

Author: Heliozoa

tmc-langs-core/src/tmc_core.rs

@@ -13,8 +13,7 @@ use reqwest::{blocking::Client, Url};
 use serde::{Deserialize, Serialize};
 use std::collections::HashMap;
 use std::io::Write;
-use std::path::Path;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::thread;
 use std::time::Duration;
 use tempfile::NamedTempFile;

tmc-langs-util/src/progress_reporter.rs

@@ -1,7 +1,8 @@
 /// Utility struct for printing progress reports.
 use serde::Serialize;
-use std::cell::Cell;
+use std::cell::RefCell;
 use std::error::Error;
+use std::sync::atomic::{AtomicUsize, Ordering};
 use std::time::Instant;
 
 /// The format for all status updates. May contain some data.
@@ -19,21 +20,23 @@ pub struct StatusUpdate<T> {
 type DynError = Box<dyn Error + Send + Sync + 'static>;
 type UpdateClosure<'a, T> = Box<dyn 'a + Fn(StatusUpdate<T>) -> Result<(), DynError>>;
 
+/// The reporter contains a RefCell for the timer, meaning care should be taken when using in a multithreaded context.
+/// The first call to progress and each step completion should be called from one thread. Other progress calls can be done from separate threads.
 pub struct ProgressReporter<'a, T> {
     progress_report: UpdateClosure<'a, T>,
-    progress_steps_total: Cell<usize>,
-    progress_steps_done: Cell<usize>,
-    start_time: Cell<Option<Instant>>,
+    progress_steps_total: AtomicUsize,
+    progress_steps_done: AtomicUsize,
+    start_time: RefCell<Option<Instant>>,
 }
 
 impl<'a, T> ProgressReporter<'a, T> {
     /// Takes a closure that will be called with all status updates, for example to print it.
     pub fn new(progress_report: impl 'a + Fn(StatusUpdate<T>) -> Result<(), DynError>) -> Self {
         Self {
             progress_report: Box::new(progress_report),
-            progress_steps_total: Cell::new(1),
-            progress_steps_done: Cell::new(0),
-            start_time: Cell::new(None),
+            progress_steps_total: AtomicUsize::new(1),
+            progress_steps_done: AtomicUsize::new(0),
+            start_time: RefCell::new(None),
         }
     }
 
@@ -42,13 +45,14 @@ impl<'a, T> ProgressReporter<'a, T> {
     /// Should be incremented to its final value before the process starts.
     pub fn increment_progress_steps(&self, amount: usize) {
         self.progress_steps_total
-            .set(self.progress_steps_total.get() + amount);
+            .fetch_add(amount, Ordering::Relaxed);
     }
 
     /// Starts the timer if not started yet.
     pub fn start_timer(&self) {
-        if self.start_time.get().is_none() {
-            self.start_time.set(Some(Instant::now()))
+        if self.start_time.borrow().is_none() {
+            let mut time = self.start_time.borrow_mut();
+            *time = Some(Instant::now())
         }
     }
 
@@ -61,30 +65,35 @@ impl<'a, T> ProgressReporter<'a, T> {
     ) -> Result<(), DynError> {
         self.start_timer();
 
-        let from_prev_steps = self.progress_steps_done.get() as f64;
-        let percent_done =
-            (from_prev_steps + step_percent_done) / self.progress_steps_total.get() as f64;
+        let from_prev_steps = self.progress_steps_done.load(Ordering::Relaxed) as f64;
+        let percent_done = (from_prev_steps + step_percent_done)
+            / self.progress_steps_total.load(Ordering::Relaxed) as f64;
 
         self.progress_report.as_ref()(StatusUpdate {
             finished: false,
             message: message.to_string(),
             percent_done,
-            time: self.start_time.get().map(|t| t.elapsed().as_millis()),
+            time: self.start_time.borrow().map(|t| t.elapsed().as_millis()),
             data,
         })
     }
 
     /// Finish the current step and the whole process if the current step is the last one.
     pub fn finish_step(&self, message: impl ToString, data: Option<T>) -> Result<(), DynError> {
-        self.progress_steps_done
-            .set(self.progress_steps_done.get() + 1);
-        if self.progress_steps_done.get() == self.progress_steps_total.get() {
+        self.progress_steps_done.fetch_add(1, Ordering::Relaxed);
+        if self.progress_steps_done.load(Ordering::Relaxed)
+            == self.progress_steps_total.load(Ordering::Relaxed)
+        {
             // all done
             let result = self.progress_report.as_ref()(StatusUpdate {
                 finished: true,
                 message: message.to_string(),
                 percent_done: 1.0,
-                time: self.start_time.take().map(|t| t.elapsed().as_millis()),
+                time: self
+                    .start_time
+                    .borrow_mut()
+                    .take()
+                    .map(|t| t.elapsed().as_millis()),
                 data,
             });
             result