From 69318465ae82281c40b68d3478232e0119dba414 Mon Sep 17 00:00:00 2001 From: Clement Tsang <34804052+ClementTsang@users.noreply.github.com> Date: Thu, 25 May 2023 00:13:04 -0400 Subject: other: simplify termination/event loop logic (#1169) This just simplifies the logic around ctrl-c and termination logic and event loop logic to something simpler and without the need for timeouts and/or atomics. Instead, we just make termination an event sent by ctrl-c and use the same receiver for event handling to react to it and break the loop. --- src/bin/main.rs | 49 +++++++++++++++++++++++-------------------------- src/lib.rs | 11 ++++++----- 2 files changed, 29 insertions(+), 31 deletions(-) diff --git a/src/bin/main.rs b/src/bin/main.rs index 9a6d1795..b7097991 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -13,10 +13,7 @@ use std::{ boxed::Box, io::stdout, panic, - sync::{ - atomic::{AtomicBool, Ordering}, - mpsc, Arc, Condvar, Mutex, - }, + sync::{mpsc, Arc, Condvar, Mutex}, thread, time::Duration, }; @@ -31,7 +28,6 @@ use tui::{backend::CrosstermBackend, Terminal}; use bottom::{ canvas::{self, canvas_styling::CanvasStyling}, - constants::*, data_conversion::*, options::*, *, @@ -87,33 +83,33 @@ fn main() -> Result<()> { // Check if the current environment is in a terminal. check_if_terminal(); - // Create termination mutex and cvar - #[allow(clippy::mutex_atomic)] - let thread_termination_lock = Arc::new(Mutex::new(false)); - let thread_termination_cvar = Arc::new(Condvar::new()); + // 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 (sender, receiver) = mpsc::channel(); - // Set up event loop thread; we set this up early to speed up first-time-to-data. + // Set up the event loop thread; we set this up early to speed up first-time-to-data. let (collection_thread_ctrl_sender, collection_thread_ctrl_receiver) = mpsc::channel(); let _collection_thread = create_collection_thread( sender.clone(), collection_thread_ctrl_receiver, - thread_termination_lock.clone(), - thread_termination_cvar.clone(), + termination_lock.clone(), + termination_cvar.clone(), &app.app_config_fields, app.filters.clone(), app.used_widgets, ); - // Set up input handling loop thread. - let _input_thread = create_input_thread(sender.clone(), thread_termination_lock.clone()); + // Set up the input handling loop thread. + let _input_thread = create_input_thread(sender.clone(), termination_lock.clone()); - // Set up cleaning loop thread. + // Set up the cleaning loop thread. let _cleaning_thread = { - let lock = thread_termination_lock.clone(); - let cvar = thread_termination_cvar.clone(); - let cleaning_sender = sender; + let lock = termination_lock.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 || { loop { @@ -163,20 +159,21 @@ fn main() -> Result<()> { panic::set_hook(Box::new(panic_hook)); // Set termination hook - let is_terminated = Arc::new(AtomicBool::new(false)); - let ist_clone = is_terminated.clone(); ctrlc::set_handler(move || { - ist_clone.store(true, Ordering::SeqCst); + let _ = sender.send(BottomEvent::Terminate); })?; + let mut first_run = true; // Draw once first to initialize the canvas, so it doesn't feel like it's frozen. try_drawing(&mut terminal, &mut app, &mut painter)?; - while !is_terminated.load(Ordering::SeqCst) { - // TODO: Would be good to instead use a mix of is_terminated check + recv. Probably use a termination event instead. - if let Ok(recv) = receiver.recv_timeout(Duration::from_millis(TICK_RATE_IN_MILLISECONDS)) { + loop { + if let Ok(recv) = receiver.recv() { match recv { + BottomEvent::Terminate => { + break; + } BottomEvent::Resize => { // FIXME: This is bugged with frozen? try_drawing(&mut terminal, &mut app, &mut painter)?; @@ -328,8 +325,8 @@ fn main() -> Result<()> { } // I think doing it in this order is safe... - *thread_termination_lock.lock().unwrap() = true; - thread_termination_cvar.notify_all(); + *termination_lock.lock().unwrap() = true; + termination_cvar.notify_all(); cleanup_terminal(&mut terminal)?; Ok(()) diff --git a/src/lib.rs b/src/lib.rs index 879b7a0b..83938c8d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -79,6 +79,7 @@ pub enum BottomEvent { PasteEvent(String), Update(Box), Clean, + Terminate, } #[derive(Debug)] @@ -482,7 +483,7 @@ pub fn create_input_thread( pub fn create_collection_thread( sender: Sender, control_receiver: Receiver, - termination_ctrl_lock: Arc>, termination_ctrl_cvar: Arc, + termination_lock: Arc>, termination_cvar: Arc, app_config_fields: &AppConfigFields, filters: DataFilters, used_widget_set: UsedWidgets, ) -> JoinHandle<()> { let temp_type = app_config_fields.temperature_type; @@ -504,7 +505,7 @@ pub fn create_collection_thread( loop { // Check once at the very top... don't block though. - if let Ok(is_terminated) = termination_ctrl_lock.try_lock() { + if let Ok(is_terminated) = termination_lock.try_lock() { if *is_terminated { drop(is_terminated); break; @@ -533,7 +534,7 @@ pub fn create_collection_thread( data_state.update_data(); // Yet another check to bail if needed... do not block! - if let Ok(is_terminated) = termination_ctrl_lock.try_lock() { + if let Ok(is_terminated) = termination_lock.try_lock() { if *is_terminated { drop(is_terminated); break; @@ -547,8 +548,8 @@ pub fn create_collection_thread( } // This is actually used as a "sleep" that can be interrupted by another thread. - if let Ok((is_terminated, _wait_timeout_result)) = termination_ctrl_cvar.wait_timeout( - termination_ctrl_lock.lock().unwrap(), + if let Ok((is_terminated, _)) = termination_cvar.wait_timeout( + termination_lock.lock().unwrap(), Duration::from_millis(update_time), ) { if *is_terminated { -- cgit v1.2.3