refactor: clean up some config and panic code (#1556)

* clean up some code

* refactor cancellation system to a separate cancellation token struct and clean up panic code

src/constants.rs

@@ -23,7 +23,7 @@ pub const TIME_LABEL_HEIGHT_LIMIT: u16 = 7;
 pub const SIDE_BORDERS: Borders = Borders::LEFT.union(Borders::RIGHT);
 
 // Help text
-pub const HELP_CONTENTS_TEXT: [&str; 10] = [
+const HELP_CONTENTS_TEXT: [&str; 10] = [
     "Either scroll or press the number key to go to the corresponding help menu section:",
     "1 - General",
     "2 - CPU widget",
@@ -38,7 +38,7 @@ pub const HELP_CONTENTS_TEXT: [&str; 10] = [
 
 // TODO [Help]: Search in help?
 // TODO [Help]: Move to using tables for easier formatting?
-pub const GENERAL_HELP_TEXT: [&str; 32] = [
+pub(crate) const GENERAL_HELP_TEXT: [&str; 32] = [
     "1 - General",
     "q, Ctrl-c        Quit",
     "Esc              Close dialog windows, search, widgets, or exit expanded mode",
@@ -73,12 +73,12 @@ pub const GENERAL_HELP_TEXT: [&str; 32] = [
     "Mouse click      Selects the clicked widget, table entry, dialog option, or tab",
 ];
 
-pub const CPU_HELP_TEXT: [&str; 2] = [
+const CPU_HELP_TEXT: [&str; 2] = [
     "2 - CPU widget",
     "Mouse scroll     Scrolling over an CPU core/average shows only that entry on the chart",
 ];
 
-pub const PROCESS_HELP_TEXT: [&str; 17] = [
+const PROCESS_HELP_TEXT: [&str; 17] = [
     "3 - Process widget",
     "dd, F9           Kill the selected process",
     "c                Sort by CPU usage, press again to reverse",
@@ -98,7 +98,7 @@ pub const PROCESS_HELP_TEXT: [&str; 17] = [
     "M                Sort by GPU memory usage, press again to reverse",
 ];
 
-pub const SEARCH_HELP_TEXT: [&str; 51] = [
+const SEARCH_HELP_TEXT: [&str; 51] = [
     "4 - Process search widget",
     "Esc              Close the search widget (retains the filter)",
     "Ctrl-a           Skip to the start of the search query",
@@ -152,7 +152,7 @@ pub const SEARCH_HELP_TEXT: [&str; 51] = [
     "TiB              ex: read > 1 tib",
 ];
 
-pub const SORT_HELP_TEXT: [&str; 6] = [
+const SORT_HELP_TEXT: [&str; 6] = [
     "5 - Sort widget",
     "Down, 'j'        Scroll down in list",
     "Up, 'k'          Scroll up in list",
@@ -161,13 +161,13 @@ pub const SORT_HELP_TEXT: [&str; 6] = [
     "Enter            Sort by current selected column",
 ];
 
-pub const TEMP_HELP_WIDGET: [&str; 3] = [
+const TEMP_HELP_WIDGET: [&str; 3] = [
     "6 - Temperature widget",
     "'s'              Sort by sensor name, press again to reverse",
     "'t'              Sort by temperature, press again to reverse",
 ];
 
-pub const DISK_HELP_WIDGET: [&str; 9] = [
+const DISK_HELP_WIDGET: [&str; 9] = [
     "7 - Disk widget",
     "'d'              Sort by disk name, press again to reverse",
     "'m'              Sort by disk mount, press again to reverse",
@@ -179,18 +179,18 @@ pub const DISK_HELP_WIDGET: [&str; 9] = [
     "'w'              Sort by disk write activity, press again to reverse",
 ];
 
-pub const BATTERY_HELP_TEXT: [&str; 3] = [
+const BATTERY_HELP_TEXT: [&str; 3] = [
     "8 - Battery widget",
     "Left             Go to previous battery",
     "Right            Go to next battery",
 ];
 
-pub const BASIC_MEM_HELP_TEXT: [&str; 2] = [
+const BASIC_MEM_HELP_TEXT: [&str; 2] = [
     "9 - Basic memory widget",
     "%                Toggle between values and percentages for memory usage",
 ];
 
-pub const HELP_TEXT: [&[&str]; HELP_CONTENTS_TEXT.len()] = [
+pub(crate) const HELP_TEXT: [&[&str]; HELP_CONTENTS_TEXT.len()] = [
     &HELP_CONTENTS_TEXT,
     &GENERAL_HELP_TEXT,
     &CPU_HELP_TEXT,
@@ -203,8 +203,7 @@ pub const HELP_TEXT: [&[&str]; HELP_CONTENTS_TEXT.len()] = [
     &BASIC_MEM_HELP_TEXT,
 ];
 
-// Default layouts
+pub(crate) const DEFAULT_LAYOUT: &str = r#"
-pub const DEFAULT_LAYOUT: &str = r#"
 [[row]]
   ratio=30
   [[row.child]]
@@ -229,7 +228,7 @@ pub const DEFAULT_LAYOUT: &str = r#"
     default=true
 "#;
 
-pub const DEFAULT_BATTERY_LAYOUT: &str = r#"
+pub(crate) const DEFAULT_BATTERY_LAYOUT: &str = r#"
 [[row]]
   ratio=30
   [[row.child]]
@@ -258,10 +257,8 @@ pub const DEFAULT_BATTERY_LAYOUT: &str = r#"
     default=true
 "#;
 
-// Config and flags
-
 // TODO: Eventually deprecate this, or grab from a file.
-pub const CONFIG_TEXT: &str = r#"# This is a default config file for bottom. All of the settings are commented
+pub(crate) const CONFIG_TEXT: &str = r#"# This is a default config file for bottom. All of the settings are commented
 # out by default; if you wish to change them uncomment and modify as you see
 # fit.
 
@@ -475,15 +472,6 @@ pub const CONFIG_TEXT: &str = r#"# This is a default config file for bottom. All
 #    default=true
 "#;
 
-pub const CONFIG_TOP_HEAD: &str = r##"# This is bottom's config file.
-# Values in this config file will change when changed in the interface.
-# You can also manually change these values.
-# Be aware that contents of this file will be overwritten if something is
-# changed in the application; you can disable writing via the
-# --no_write flag or no_write config option.
-
-"##;
-
 #[cfg(test)]
 mod test {
     use super::*;

src/data_conversion.rs

@@ -74,8 +74,8 @@ pub struct ConvertedData {
     pub cache_labels: Option<(String, String)>,
     pub swap_labels: Option<(String, String)>,
 
-    pub mem_data: Vec<Point>, /* TODO: Switch this and all data points over to a better data
+    // TODO: Switch this and all data points over to a better data structure.
-                               * structure... */
+    pub mem_data: Vec<Point>,
     #[cfg(not(target_os = "windows"))]
     pub cache_data: Vec<Point>,
     pub swap_data: Vec<Point>,

src/main.rs

@@ -9,6 +9,7 @@
 
 pub mod app;
 pub mod utils {
+    pub mod cancellation_token;
     pub mod data_prefixes;
     pub mod data_units;
     pub mod general;
@@ -29,7 +30,7 @@ use std::{
     panic::{self, PanicInfo},
     sync::{
         mpsc::{self, Receiver, Sender},
-        Arc, Condvar, Mutex,
+        Arc,
     },
     thread::{self, JoinHandle},
     time::{Duration, Instant},
@@ -49,6 +50,7 @@ use data_conversion::*;
 use event::{handle_key_event_or_break, handle_mouse_event, BottomEvent, CollectionThreadEvent};
 use options::{args, get_or_create_config, init_app};
 use tui::{backend::CrosstermBackend, Terminal};
+use utils::cancellation_token::CancellationToken;
 #[allow(unused_imports)]
 use utils::logging::*;
 
@@ -131,23 +133,27 @@ fn panic_hook(panic_info: &PanicInfo<'_>) {
             )),
         );
     }
+
+    // TODO: Might be cleaner in the future to use a cancellation token, but that causes some fun issues with
+    // lifetimes; for now if it panics then shut down the main program entirely ASAP.
+    std::process::exit(1);
 }
 
 /// Create a thread to poll for user inputs and forward them to the main thread.
 fn create_input_thread(
-    sender: Sender<BottomEvent>, termination_ctrl_lock: Arc<Mutex<bool>>,
+    sender: Sender<BottomEvent>, cancellation_token: Arc<CancellationToken>,
 ) -> JoinHandle<()> {
     thread::spawn(move || {
         let mut mouse_timer = Instant::now();
 
         loop {
-            if let Ok(is_terminated) = termination_ctrl_lock.try_lock() {
+            // We don't block.
-                // We don't block.
+            if let Some(is_terminated) = cancellation_token.try_check() {
-                if *is_terminated {
+                if is_terminated {
-                    drop(is_terminated);
                     break;
                 }
             }
+
             if let Ok(poll) = poll(Duration::from_millis(20)) {
                 if poll {
                     if let Ok(event) = read() {
@@ -206,8 +212,8 @@ fn create_input_thread(
 /// Create a thread to handle data collection.
 fn create_collection_thread(
     sender: Sender<BottomEvent>, control_receiver: Receiver<CollectionThreadEvent>,
-    termination_lock: Arc<Mutex<bool>>, termination_cvar: Arc<Condvar>,
+    cancellation_token: Arc<CancellationToken>, app_config_fields: &AppConfigFields,
-    app_config_fields: &AppConfigFields, filters: DataFilters, used_widget_set: UsedWidgets,
+    filters: DataFilters, used_widget_set: UsedWidgets,
 ) -> JoinHandle<()> {
     let temp_type = app_config_fields.temperature_type;
     let use_current_cpu_total = app_config_fields.use_current_cpu_total;
@@ -228,9 +234,8 @@ fn create_collection_thread(
 
         loop {
             // Check once at the very top... don't block though.
-            if let Ok(is_terminated) = termination_lock.try_lock() {
+            if let Some(is_terminated) = cancellation_token.try_check() {
-                if *is_terminated {
+                if is_terminated {
-                    drop(is_terminated);
                     break;
                 }
             }
@@ -247,9 +252,8 @@ fn create_collection_thread(
             data_state.update_data();
 
             // Yet another check to bail if needed... do not block!
-            if let Ok(is_terminated) = termination_lock.try_lock() {
+            if let Some(is_terminated) = cancellation_token.try_check() {
-                if *is_terminated {
+                if is_terminated {
-                    drop(is_terminated);
                     break;
                 }
             }
@@ -260,15 +264,9 @@ fn create_collection_thread(
                 break;
             }
 
-            // This is actually used as a "sleep" that can be interrupted by another thread.
+            // Sleep while allowing for interruptions...
-            if let Ok((is_terminated, _)) = termination_cvar.wait_timeout(
+            if cancellation_token.sleep_with_cancellation(Duration::from_millis(update_time)) {
-                termination_lock.lock().unwrap(),
+                break;
-                Duration::from_millis(update_time),
-            ) {
-                if *is_terminated {
-                    drop(is_terminated);
-                    break;
-                }
             }
         }
     })
@@ -308,8 +306,6 @@ fn generate_schema() -> anyhow::Result<()> {
 }
 
 fn main() -> anyhow::Result<()> {
-    // TODO: If there is any panic in any thread, send a cancellation token (or similar) to shut down everything.
-
     // let _profiler = dhat::Profiler::new_heap();
 
     let args = args::get_args();
@@ -341,12 +337,7 @@ fn main() -> anyhow::Result<()> {
     // Check if the current environment is in a terminal.
     check_if_terminal();
 
-    // Create termination mutex and cvar. We use this setup because we need to sleep
+    let cancellation_token = Arc::new(CancellationToken::default());
-    // at some points in the update thread, but we want to be able to interrupt
-    // the "sleep" if a termination occurs.
-    let termination_lock = Arc::new(Mutex::new(false));
-    let termination_cvar = Arc::new(Condvar::new());
-
     let (sender, receiver) = mpsc::channel();
 
     // Set up the event loop thread; we set this up early to speed up
@@ -355,37 +346,27 @@ fn main() -> anyhow::Result<()> {
     let _collection_thread = create_collection_thread(
         sender.clone(),
         collection_thread_ctrl_receiver,
-        termination_lock.clone(),
+        cancellation_token.clone(),
-        termination_cvar.clone(),
         &app.app_config_fields,
         app.filters.clone(),
         app.used_widgets,
     );
 
     // Set up the input handling loop thread.
-    let _input_thread = create_input_thread(sender.clone(), termination_lock.clone());
+    let _input_thread = create_input_thread(sender.clone(), cancellation_token.clone());
 
     // Set up the cleaning loop thread.
     let _cleaning_thread = {
-        let lock = termination_lock.clone();
+        let cancellation_token = cancellation_token.clone();
-        let cvar = termination_cvar.clone();
         let cleaning_sender = sender.clone();
         let offset_wait_time = app.app_config_fields.retention_ms + 60000;
-        thread::spawn(move || {
+        thread::spawn(move || loop {
-            loop {
+            if cancellation_token.sleep_with_cancellation(Duration::from_millis(offset_wait_time)) {
-                let result = cvar.wait_timeout(
+                break;
-                    lock.lock().unwrap(),
+            }
-                    Duration::from_millis(offset_wait_time),
+
-                );
+            if cleaning_sender.send(BottomEvent::Clean).is_err() {
-                if let Ok(result) = result {
+                break;
-                    if *(result.0) {
-                        break;
-                    }
-                }
-                if cleaning_sender.send(BottomEvent::Clean).is_err() {
-                    // debug!("Failed to send cleaning sender...");
-                    break;
-                }
             }
         })
     };
@@ -585,8 +566,8 @@ fn main() -> anyhow::Result<()> {
     }
 
     // I think doing it in this order is safe...
-    *termination_lock.lock().unwrap() = true;
+    // TODO: maybe move the cancellation token to the ctrl-c handler?
-    termination_cvar.notify_all();
+    cancellation_token.cancel();
     cleanup_terminal(&mut terminal)?;
 
     Ok(())

src/utils/cancellation_token.rs

@@ -0,0 +1,61 @@
+use std::{
+    sync::{Condvar, Mutex},
+    time::Duration,
+};
+
+/// A cancellation token.
+pub(crate) struct CancellationToken {
+    // The "check" for the cancellation token. Setting this to true will mark the cancellation token as "cancelled".
+    mutex: Mutex<bool>,
+    cvar: Condvar,
+}
+
+impl Default for CancellationToken {
+    fn default() -> Self {
+        Self {
+            mutex: Mutex::new(false),
+            cvar: Condvar::new(),
+        }
+    }
+}
+
+impl CancellationToken {
+    /// Mark the [`CancellationToken`] as cancelled.
+    ///
+    /// This is idempotent, and once cancelled, will stay cancelled. Sending it
+    /// again will not do anything.
+    pub fn cancel(&self) {
+        let mut guard = self
+            .mutex
+            .lock()
+            .expect("cancellation token lock should not be poisoned");
+
+        if !*guard {
+            *guard = true;
+            self.cvar.notify_all();
+        }
+    }
+
+    /// Try and check the [`CancellationToken`]'s status. Note that
+    /// this will not block.
+    pub fn try_check(&self) -> Option<bool> {
+        self.mutex.try_lock().ok().map(|guard| *guard)
+    }
+
+    /// Allows a thread to sleep while still being interruptible with by the token.
+    ///
+    /// Returns the condition state after either sleeping or being woken up.
+    pub fn sleep_with_cancellation(&self, duration: Duration) -> bool {
+        let guard = self
+            .mutex
+            .lock()
+            .expect("cancellation token lock should not be poisoned");
+
+        let (result, _) = self
+            .cvar
+            .wait_timeout(guard, duration)
+            .expect("cancellation token lock should not be poisoned");
+
+        *result
+    }
+}