Fix shutdown of config monitor

This implements a coordinated shutdown of the config monitor by sending
an event to its thread and waiting for the thread to terminate.

alacritty/src/config/monitor.rs

@@ -1,9 +1,13 @@
 use std::path::PathBuf;
-use std::sync::mpsc::{self, RecvTimeoutError};
+use std::sync::mpsc::{self, RecvTimeoutError, Sender};
+use std::thread::JoinHandle;
 use std::time::{Duration, Instant};
 
-use log::{debug, error};
-use notify::{Config, EventKind, RecommendedWatcher, RecursiveMode, Watcher};
+use log::{debug, error, warn};
+use notify::{
+ Config, Error as NotifyError, Event as NotifyEvent, EventKind, RecommendedWatcher,
+ RecursiveMode, Watcher,
+};
 use winit::event_loop::EventLoopProxy;
 
 use alacritty_terminal::thread;
@@ -15,115 +19,139 @@ const DEBOUNCE_DELAY: Duration = Duration::from_millis(10);
 /// The fallback for `RecommendedWatcher` polling.
 const FALLBACK_POLLING_TIMEOUT: Duration = Duration::from_secs(1);
 
-pub fn watch(mut paths: Vec<PathBuf>, event_proxy: EventLoopProxy<Event>) {
- // Don't monitor config if there is no path to watch.
- if paths.is_empty() {
-  return;
- }
+/// Config file update monitor.
+pub struct ConfigMonitor {
+ thread: JoinHandle<()>,
+ shutdown_tx: Sender<Result<NotifyEvent, NotifyError>>,
+}
 
- // Exclude char devices like `/dev/null`, sockets, and so on, by checking that file type is a
- // regular file.
- paths.retain(|path| {
- // Call `metadata` to resolve symbolic links.
- path.metadata().map_or(false, |metadata| metadata.file_type().is_file())
- });
-
- // Canonicalize paths, keeping the base paths for symlinks.
- for i in 0..paths.len() {
- if let Ok(canonical_path) = paths[i].canonicalize() {
- match paths[i].symlink_metadata() {
- Ok(metadata) if metadata.file_type().is_symlink() => paths.push(canonical_path),
- _ => paths[i] = canonical_path,
- }
+impl ConfigMonitor {
+ pub fn new(mut paths: Vec<PathBuf>, event_proxy: EventLoopProxy<Event>) -> Option<Self> {
+ // Don't monitor config if there is no path to watch.
+ if paths.is_empty() {
+ return None;
  }
- }
 
- // The Duration argument is a debouncing period.
- let (tx, rx) = mpsc::channel();
- let mut watcher = match RecommendedWatcher::new(
- tx,
- Config::default().with_poll_interval(FALLBACK_POLLING_TIMEOUT),
- ) {
- Ok(watcher) => watcher,
- Err(err) => {
- error!("Unable to watch config file: {}", err);
- return;
- },
- };
-
- thread::spawn_named("config watcher", move || {
- // Get all unique parent directories.
- let mut parents = paths
- .iter()
- .map(|path| {
- let mut path = path.clone();
- path.pop();
- path
- })
- .collect::<Vec<PathBuf>>();
- parents.sort_unstable();
- parents.dedup();
-
- // Watch all configuration file directories.
- for parent in &parents {
- if let Err(err) = watcher.watch(parent, RecursiveMode::NonRecursive) {
- debug!("Unable to watch config directory {:?}: {}", parent, err);
+ // Exclude char devices like `/dev/null`, sockets, and so on, by checking that file type is
+ // a regular file.
+ paths.retain(|path| {
+ // Call `metadata` to resolve symbolic links.
+ path.metadata().map_or(false, |metadata| metadata.file_type().is_file())
+ });
+
+ // Canonicalize paths, keeping the base paths for symlinks.
+ for i in 0..paths.len() {
+ if let Ok(canonical_path) = paths[i].canonicalize() {
+ match paths[i].symlink_metadata() {
+ Ok(metadata) if metadata.file_type().is_symlink() => paths.push(canonical_path),
+ _ => paths[i] = canonical_path,
+ }
  }
  }
 
- // The current debouncing time.
- let mut debouncing_deadline: Option<Instant> = None;
-
- // The events accumulated during the debounce period.
- let mut received_events = Vec::new();
-
- loop {
- // We use `recv_timeout` to debounce the events coming from the watcher and reduce
- // the amount of config reloads.
- let event = match debouncing_deadline.as_ref() {
- Some(debouncing_deadline) => {
- rx.recv_timeout(debouncing_deadline.saturating_duration_since(Instant::now()))
- },
- None => {
- let event = rx.recv().map_err(Into::into);
- // Set the debouncing deadline after receiving the event.
- debouncing_deadline = Some(Instant::now() + DEBOUNCE_DELAY);
- event
- },
- };
-
- match event {
- Ok(Ok(event)) => match event.kind {
- EventKind::Any
- | EventKind::Create(_)
- | EventKind::Modify(_)
- | EventKind::Other => {
- received_events.push(event);
+ // The Duration argument is a debouncing period.
+ let (tx, rx) = mpsc::channel();
+ let mut watcher = match RecommendedWatcher::new(
+ tx.clone(),
+ Config::default().with_poll_interval(FALLBACK_POLLING_TIMEOUT),
+ ) {
+ Ok(watcher) => watcher,
+ Err(err) => {
+ error!("Unable to watch config file: {}", err);
+ return None;
+ },
+ };
+
+ let join_handle = thread::spawn_named("config watcher", move || {
+ // Get all unique parent directories.
+ let mut parents = paths
+ .iter()
+ .map(|path| {
+ let mut path = path.clone();
+ path.pop();
+ path
+ })
+ .collect::<Vec<PathBuf>>();
+ parents.sort_unstable();
+ parents.dedup();
+
+ // Watch all configuration file directories.
+ for parent in &parents {
+ if let Err(err) = watcher.watch(parent, RecursiveMode::NonRecursive) {
+ debug!("Unable to watch config directory {:?}: {}", parent, err);
+ }
+ }
+
+ // The current debouncing time.
+ let mut debouncing_deadline: Option<Instant> = None;
+
+ // The events accumulated during the debounce period.
+ let mut received_events = Vec::new();
+
+ loop {
+ // We use `recv_timeout` to debounce the events coming from the watcher and reduce
+ // the amount of config reloads.
+ let event = match debouncing_deadline.as_ref() {
+ Some(debouncing_deadline) => rx.recv_timeout(
+ debouncing_deadline.saturating_duration_since(Instant::now()),
+ ),
+ None => {
+ let event = rx.recv().map_err(Into::into);
+ // Set the debouncing deadline after receiving the event.
+ debouncing_deadline = Some(Instant::now() + DEBOUNCE_DELAY);
+ event
+ },
+ };
+
+ match event {
+ Ok(Ok(event)) => match event.kind {
+ EventKind::Other if event.info() == Some("shutdown") => break,
+ EventKind::Any
+ | EventKind::Create(_)
+ | EventKind::Modify(_)
+ | EventKind::Other => {
+ received_events.push(event);
+ },
+ _ => (),
+ },
+ Err(RecvTimeoutError::Timeout) => {
+ // Go back to polling the events.
+ debouncing_deadline = None;
+
+ if received_events
+ .drain(..)
+ .flat_map(|event| event.paths.into_iter())
+ .any(|path| paths.contains(&path))
+ {
+ // Always reload the primary configuration file.
+ let event = Event::new(EventType::ConfigReload(paths[0].clone()), None);
+ let _ = event_proxy.send_event(event);
+ }
  },
- _ => (),
- },
- Err(RecvTimeoutError::Timeout) => {
- // Go back to polling the events.
- debouncing_deadline = None;
-
- if received_events
-  .drain(..)
-  .flat_map(|event| event.paths.into_iter())
-  .any(|path| paths.contains(&path))
- {
-  // Always reload the primary configuration file.
-  let event = Event::new(EventType::ConfigReload(paths[0].clone()), None);
- let _ = event_proxy.send_event(event);
-  }
-  },
-  Ok(Err(err)) => {
-  debug!("Config watcher errors: {:?}", err);
-  },
-  Err(err) => {
- debug!("Config watcher channel dropped unexpectedly: {}", err);
-  break;
-  },
- };
+ Ok(Err(err)) => {
+  debug!("Config watcher errors: {:?}", err);
+  },
+ Err(err) => {
+  debug!("Config watcher channel dropped unexpectedly: {}", err);
+ break;
+ },
+ };
+ }
+ });
+
+ Some(Self { thread: join_handle, shutdown_tx: tx })
+ }
+
+ /// Synchronously shut down the monitor.
+ pub fn shutdown(self) {
+ // Request shutdown.
+ let mut event = NotifyEvent::new(EventKind::Other);
+ event = event.set_info("shutdown");
+ let _ = self.shutdown_tx.send(Ok(event));
+
+ // Wait for thread to terminate.
+ if let Err(err) = self.thread.join() {
+ warn!("config monitor shutdown failed: {err:?}");
  }
- });
+ }
 }

alacritty/src/main.rs

@@ -56,7 +56,8 @@ mod gl {
 #[cfg(unix)]
 use crate::cli::MessageOptions;
 use crate::cli::{Options, Subcommands};
-use crate::config::{monitor, UiConfig};
+use crate::config::monitor::ConfigMonitor;
+use crate::config::UiConfig;
 use crate::event::{Event, Processor};
 #[cfg(target_os = "macos")]
 use crate::macos::locale;
@@ -165,8 +166,10 @@ fn alacritty(mut options: Options) -> Result<(), Box<dyn Error>> {
  //
  // The monitor watches the config file for changes and reloads it. Pending
  // config changes are processed in the main loop.
+ let mut config_monitor = None;
  if config.live_config_reload {
- monitor::watch(config.config_paths.clone(), window_event_loop.create_proxy());
+ config_monitor =
+ ConfigMonitor::new(config.config_paths.clone(), window_event_loop.create_proxy());
  }
 
  // Create the IPC socket listener.
@@ -205,8 +208,10 @@ fn alacritty(mut options: Options) -> Result<(), Box<dyn Error>> {
  // FIXME: Change PTY API to enforce the correct drop order with the typesystem.
  drop(processor);
 
- // FIXME patch notify library to have a shutdown method.
- // config_reloader.join().ok();
+ // Terminate the config monitor.
+ if let Some(config_monitor) = config_monitor.take() {
+ config_monitor.shutdown();
+ }
 
  // Without explicitly detaching the console cmd won't redraw it's prompt.
  #[cfg(windows)]