Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix shutdown of config monitor #7936

Merged
merged 1 commit into from May 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
240 changes: 134 additions & 106 deletions 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;
Expand All @@ -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:?}");
chrisduerr marked this conversation as resolved.
Show resolved Hide resolved
}
});
}
}
13 changes: 9 additions & 4 deletions alacritty/src/main.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)]
Expand Down