Skip to content

Commit

Permalink
refactor: clean up some config and panic code (#1556)
Browse files Browse the repository at this point in the history 
* clean up some code

* refactor cancellation system to a separate cancellation token struct and clean up panic code
  • Loading branch information
@ClementTsang
ClementTsang authored Aug 11, 2024
1 parent 96ed26d commit c65121c
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 81 deletions.
40 changes: 14 additions & 26 deletions src/constants.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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,
Expand All @@ -203,8 +203,7 @@ pub const HELP_TEXT: [&[&str]; HELP_CONTENTS_TEXT.len()] = [
&BASIC_MEM_HELP_TEXT,
];

// Default layouts
pub const DEFAULT_LAYOUT: &str = r#"
pub(crate) const DEFAULT_LAYOUT: &str = r#"
[[row]]
ratio=30
[[row.child]]
Expand All @@ -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]]
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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::*;
Expand Down
4 changes: 2 additions & 2 deletions src/data_conversion.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
* structure... */
// TODO: Switch this and all data points over to a better data structure.
pub mem_data: Vec<Point>,
#[cfg(not(target_os = "windows"))]
pub cache_data: Vec<Point>,
pub swap_data: Vec<Point>,
Expand Down
87 changes: 34 additions & 53 deletions src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -29,7 +30,7 @@ use std::{
panic::{self, PanicInfo},
sync::{
mpsc::{self, Receiver, Sender},
Arc, Condvar, Mutex,
Arc,
},
thread::{self, JoinHandle},
time::{Duration, Instant},
Expand All @@ -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::*;

Expand Down Expand Up @@ -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.
if *is_terminated {
drop(is_terminated);
// We don't block.
if let Some(is_terminated) = cancellation_token.try_check() {
if is_terminated {
break;
}
}

if let Ok(poll) = poll(Duration::from_millis(20)) {
if poll {
if let Ok(event) = read() {
Expand Down Expand Up @@ -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>,
app_config_fields: &AppConfigFields, filters: DataFilters, used_widget_set: UsedWidgets,
cancellation_token: Arc<CancellationToken>, app_config_fields: &AppConfigFields,
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;
Expand All @@ -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 *is_terminated {
drop(is_terminated);
if let Some(is_terminated) = cancellation_token.try_check() {
if is_terminated {
break;
}
}
Expand All @@ -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 *is_terminated {
drop(is_terminated);
if let Some(is_terminated) = cancellation_token.try_check() {
if is_terminated {
break;
}
}
Expand All @@ -260,15 +264,9 @@ fn create_collection_thread(
break;
}

// This is actually used as a "sleep" that can be interrupted by another thread.
if let Ok((is_terminated, _)) = termination_cvar.wait_timeout(
termination_lock.lock().unwrap(),
Duration::from_millis(update_time),
) {
if *is_terminated {
drop(is_terminated);
break;
}
// Sleep while allowing for interruptions...
if cancellation_token.sleep_with_cancellation(Duration::from_millis(update_time)) {
break;
}
}
})
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
// 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 cancellation_token = Arc::new(CancellationToken::default());
let (sender, receiver) = mpsc::channel();

// Set up the event loop thread; we set this up early to speed up
Expand All @@ -355,37 +346,27 @@ fn main() -> anyhow::Result<()> {
let _collection_thread = create_collection_thread(
sender.clone(),
collection_thread_ctrl_receiver,
termination_lock.clone(),
termination_cvar.clone(),
cancellation_token.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 cvar = termination_cvar.clone();
let cancellation_token = cancellation_token.clone();
let cleaning_sender = sender.clone();
let offset_wait_time = app.app_config_fields.retention_ms + 60000;
thread::spawn(move || {
loop {
let result = cvar.wait_timeout(
lock.lock().unwrap(),
Duration::from_millis(offset_wait_time),
);
if let Ok(result) = result {
if *(result.0) {
break;
}
}
if cleaning_sender.send(BottomEvent::Clean).is_err() {
// debug!("Failed to send cleaning sender...");
break;
}
thread::spawn(move || loop {
if cancellation_token.sleep_with_cancellation(Duration::from_millis(offset_wait_time)) {
break;
}

if cleaning_sender.send(BottomEvent::Clean).is_err() {
break;
}
})
};
Expand Down Expand Up @@ -585,8 +566,8 @@ fn main() -> anyhow::Result<()> {
}

// I think doing it in this order is safe...
*termination_lock.lock().unwrap() = true;
termination_cvar.notify_all();
// TODO: maybe move the cancellation token to the ctrl-c handler?
cancellation_token.cancel();
cleanup_terminal(&mut terminal)?;

Ok(())
Expand Down
Loading

0 comments on commit c65121c

Please sign in to comment.