From 4046a1c6c0d25ea330b8376690d6204b5ad406b8 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Mon, 22 Jun 2026 12:59:57 +0200 Subject: [PATCH 1/7] refactor: extract Executor struct, remove CommandSucceeded from channel The Event enum previously carried both watcher events (ChangeDetected) and build-completion signals (CommandSucceeded) on the same mpsc channel. Extract a dedicated Executor struct that owns the build thread and lets the main loop poll for completion via JoinHandle. This removes the conceptual dual-role of Event and eliminates the channel round-trip for lock-release signalling. - Introduce Executor with spawn/cancel/take_result methods - Rename Event to WatchEvent (only ChangeDetected remains) - Refactor run() to poll exec.take_result() instead of receiving CommandSucceeded through the watcher channel - No functional change --- src/lib.rs | 155 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 104 insertions(+), 51 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 895bb03..9afb513 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -304,9 +304,8 @@ impl Watch { } } - let mut current_child = SharedChild::new(); + let mut exec = Executor::new(); let mut lock_guard = Some(self.watch_lock.write()); - let mut generation: u64 = 0; // `pending_build` tracks whether a change has arrived that has not yet // been translated into a spawned command. It starts as `true` so the @@ -320,48 +319,35 @@ impl Watch { let mut has_file_changes = false; loop { + // Poll for build completion before acting on pending state. + if let Some(succeeded) = exec.take_result() { + if succeeded { + log::trace!("Command succeeded, releasing lock"); + lock_guard.take(); + } + } + if pending_build { pending_build = false; has_file_changes = false; log::info!("Running command"); - let mut current_child = current_child.clone(); - let mut list = list.clone(); - let tx = tx.clone(); - let build_id = generation; - thread::spawn(move || { - let mut status = ExitStatus::default(); - - list.spawn(|res| match res { - Err(err) => { - log::error!("Could not execute command: {err}"); - false - } - Ok(child) => { - log::trace!("Child spawned PID: {}", child.id()); - current_child.replace(child); - status = current_child.wait(); - status.success() - } - }); - - if status.success() { - log::info!("Command succeeded."); - tx.send(Event::CommandSucceeded(build_id)) - .expect("can send"); - } else if let Some(code) = status.code() { - log::error!("Command failed (exit code: {code})"); - } else { - log::error!("Command failed."); - } - }); + exec.spawn(list.clone()); } // Drain all events that arrive within the debounce window. Each // new event resets the timer; we only (re)build once things have // been quiet for `debounce`. loop { + // Poll for build completion between debounce waits. + if let Some(succeeded) = exec.take_result() { + if succeeded { + log::trace!("Command succeeded, releasing lock"); + lock_guard.take(); + } + } + match rx.recv_timeout(self.debounce) { - Ok(Event::ChangeDetected { git }) => { + Ok(WatchEvent::ChangeDetected { git }) => { if git { log::trace!("Git directory change detected, resetting debounce timer"); } else { @@ -369,25 +355,22 @@ impl Watch { has_file_changes = true; } if !pending_build { - current_child.terminate(); - generation += 1; + exec.cancel(); if lock_guard.is_none() { lock_guard = Some(self.watch_lock.write()); } pending_build = true; } } - Ok(Event::CommandSucceeded(build_id)) if build_id == generation => { - log::trace!("Command succeeded, releasing lock"); - lock_guard.take(); - // Continue waiting for the next change. - } - Ok(Event::CommandSucceeded(build_id)) => { - log::trace!( - "Ignoring stale success from build {build_id} (current: {generation})" - ); - } Err(mpsc::RecvTimeoutError::Timeout) => { + // Poll for build completion that finished during the wait. + if let Some(succeeded) = exec.take_result() { + if succeeded { + log::trace!("Command succeeded, releasing lock"); + lock_guard.take(); + } + } + // Quiet for `debounce` — time to build if there is a // pending change. if pending_build { @@ -418,7 +401,7 @@ impl Watch { } } Err(mpsc::RecvTimeoutError::Disconnected) => { - current_child.terminate(); + exec.cancel(); return Ok(()); } } @@ -559,7 +542,7 @@ impl Watch { struct WatchEventHandler { watch: Watch, - tx: mpsc::Sender, + tx: mpsc::Sender, git_dirs: Vec, } @@ -584,14 +567,14 @@ impl notify::EventHandler for WatchEventHandler { { log::trace!("Git directory change detected in {event:?}"); self.tx - .send(Event::ChangeDetected { git: true }) + .send(WatchEvent::ChangeDetected { git: true }) .expect("can send"); return; } log::trace!("Changes detected in {event:?}"); self.tx - .send(Event::ChangeDetected { git: false }) + .send(WatchEvent::ChangeDetected { git: false }) .expect("can send"); } else { log::trace!("Ignoring non-create/modify event: {event:?}"); @@ -684,6 +667,77 @@ impl SharedChild { } } +/// Encapsulates a build running on a background thread. +/// +/// Tracks the child process, generation counter, and [`JoinHandle`] so the +/// main loop can poll for completion without a dedicated channel. +struct Executor { + child: SharedChild, + build_handle: Option>, + generation: u64, +} + +impl Executor { + fn new() -> Self { + Self { + child: SharedChild::new(), + build_handle: None, + generation: 0, + } + } + + /// Spawn a build on a background thread. + fn spawn(&mut self, mut commands: CommandList) { + let mut child = self.child.clone(); + + self.build_handle = Some(thread::spawn(move || { + let mut status = ExitStatus::default(); + + commands.spawn(|res| match res { + Err(err) => { + log::error!("Could not execute command: {err}"); + false + } + Ok(process) => { + log::trace!("Child spawned PID: {}", process.id()); + child.replace(process); + status = child.wait(); + status.success() + } + }); + + if status.success() { + log::info!("Command succeeded."); + } else if let Some(code) = status.code() { + log::error!("Command failed (exit code: {code})"); + } else { + log::error!("Command failed."); + } + + status.success() + })); + } + + /// Terminate the current build (if any) and bump the generation so + /// the next [`spawn`](Self::spawn) starts fresh. + fn cancel(&mut self) { + self.child.terminate(); + self.generation += 1; + self.build_handle.take(); + } + + /// If the current build has finished, return `Some(success)`, + /// otherwise return `None`. + fn take_result(&mut self) -> Option { + let handle = self.build_handle.as_ref()?; + if handle.is_finished() { + Some(self.build_handle.take().unwrap().join().unwrap_or(false)) + } else { + None + } + } +} + /// A list of commands to run. #[derive(Debug, Clone)] pub struct CommandList { @@ -800,8 +854,7 @@ impl WatchLock { } #[derive(Debug)] -enum Event { - CommandSucceeded(u64), +enum WatchEvent { ChangeDetected { git: bool }, } From a0dea51e08133404c3a12b92d4811ea924ad9632 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Mon, 22 Jun 2026 14:31:12 +0200 Subject: [PATCH 2/7] chore: remove redundant take_result() polls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only the Timeout branch poll is needed — builds that complete during the debounce wait are caught there. The outer-loop and inner-loop-top polls always see None. --- src/lib.rs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 9afb513..03ecedc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -319,14 +319,6 @@ impl Watch { let mut has_file_changes = false; loop { - // Poll for build completion before acting on pending state. - if let Some(succeeded) = exec.take_result() { - if succeeded { - log::trace!("Command succeeded, releasing lock"); - lock_guard.take(); - } - } - if pending_build { pending_build = false; has_file_changes = false; @@ -338,14 +330,6 @@ impl Watch { // new event resets the timer; we only (re)build once things have // been quiet for `debounce`. loop { - // Poll for build completion between debounce waits. - if let Some(succeeded) = exec.take_result() { - if succeeded { - log::trace!("Command succeeded, releasing lock"); - lock_guard.take(); - } - } - match rx.recv_timeout(self.debounce) { Ok(WatchEvent::ChangeDetected { git }) => { if git { From b74e2aabbf11916cc350d7d5e1fd600977c126e4 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 23 Jun 2026 09:30:07 +0200 Subject: [PATCH 3/7] fix: remove dead generation field, add BuildFinished wake-up nudge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove generation from Executor — staleness handled by dropping JoinHandle on cancel(). take_result() only sees the current build. - Add WatchEvent::BuildFinished variant sent from build thread on exit. Wakes recv_timeout immediately so lock is released without waiting for the debounce timeout. --- src/lib.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 03ecedc..375222b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -323,7 +323,7 @@ impl Watch { pending_build = false; has_file_changes = false; log::info!("Running command"); - exec.spawn(list.clone()); + exec.spawn(list.clone(), tx.clone()); } // Drain all events that arrive within the debounce window. Each @@ -346,6 +346,14 @@ impl Watch { pending_build = true; } } + Ok(WatchEvent::BuildFinished) => { + if let Some(succeeded) = exec.take_result() { + if succeeded { + log::trace!("Command succeeded, releasing lock"); + lock_guard.take(); + } + } + } Err(mpsc::RecvTimeoutError::Timeout) => { // Poll for build completion that finished during the wait. if let Some(succeeded) = exec.take_result() { @@ -658,7 +666,6 @@ impl SharedChild { struct Executor { child: SharedChild, build_handle: Option>, - generation: u64, } impl Executor { @@ -666,12 +673,11 @@ impl Executor { Self { child: SharedChild::new(), build_handle: None, - generation: 0, } } /// Spawn a build on a background thread. - fn spawn(&mut self, mut commands: CommandList) { + fn spawn(&mut self, mut commands: CommandList, tx: mpsc::Sender) { let mut child = self.child.clone(); self.build_handle = Some(thread::spawn(move || { @@ -698,15 +704,14 @@ impl Executor { log::error!("Command failed."); } + let _ = tx.send(WatchEvent::BuildFinished); status.success() })); } - /// Terminate the current build (if any) and bump the generation so - /// the next [`spawn`](Self::spawn) starts fresh. + /// Terminate the current build (if any). fn cancel(&mut self) { self.child.terminate(); - self.generation += 1; self.build_handle.take(); } @@ -840,6 +845,7 @@ impl WatchLock { #[derive(Debug)] enum WatchEvent { ChangeDetected { git: bool }, + BuildFinished, } #[cfg(test)] From cb76c91f3aa2c76e850eac4a5b3ec701e232b1ff Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 23 Jun 2026 09:30:45 +0200 Subject: [PATCH 4/7] chore: move Sender into Executor constructor --- src/lib.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 375222b..ac7a273 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -304,7 +304,7 @@ impl Watch { } } - let mut exec = Executor::new(); + let mut exec = Executor::new(tx.clone()); let mut lock_guard = Some(self.watch_lock.write()); // `pending_build` tracks whether a change has arrived that has not yet @@ -323,7 +323,7 @@ impl Watch { pending_build = false; has_file_changes = false; log::info!("Running command"); - exec.spawn(list.clone(), tx.clone()); + exec.spawn(list.clone()); } // Drain all events that arrive within the debounce window. Each @@ -666,19 +666,22 @@ impl SharedChild { struct Executor { child: SharedChild, build_handle: Option>, + tx: mpsc::Sender, } impl Executor { - fn new() -> Self { + fn new(tx: mpsc::Sender) -> Self { Self { child: SharedChild::new(), build_handle: None, + tx, } } /// Spawn a build on a background thread. - fn spawn(&mut self, mut commands: CommandList, tx: mpsc::Sender) { + fn spawn(&mut self, mut commands: CommandList) { let mut child = self.child.clone(); + let tx = self.tx.clone(); self.build_handle = Some(thread::spawn(move || { let mut status = ExitStatus::default(); From f180e97859efd161b84d861b1a61908fa055a827 Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 23 Jun 2026 09:33:34 +0200 Subject: [PATCH 5/7] chore: replace Sender with Arc in Executor Executor no longer depends on mpsc. The wake-up mechanism is abstracted behind a plain callable. --- src/lib.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ac7a273..7426740 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -304,7 +304,9 @@ impl Watch { } } - let mut exec = Executor::new(tx.clone()); + let mut exec = Executor::new(Arc::new(move || { + let _ = tx.send(WatchEvent::BuildFinished); + })); let mut lock_guard = Some(self.watch_lock.write()); // `pending_build` tracks whether a change has arrived that has not yet @@ -666,22 +668,22 @@ impl SharedChild { struct Executor { child: SharedChild, build_handle: Option>, - tx: mpsc::Sender, + notify: Arc, } impl Executor { - fn new(tx: mpsc::Sender) -> Self { + fn new(notify: Arc) -> Self { Self { child: SharedChild::new(), build_handle: None, - tx, + notify, } } /// Spawn a build on a background thread. fn spawn(&mut self, mut commands: CommandList) { let mut child = self.child.clone(); - let tx = self.tx.clone(); + let notify = self.notify.clone(); self.build_handle = Some(thread::spawn(move || { let mut status = ExitStatus::default(); @@ -707,7 +709,7 @@ impl Executor { log::error!("Command failed."); } - let _ = tx.send(WatchEvent::BuildFinished); + notify(); status.success() })); } From c3bf1e28e6f46a270994c0b17ca76c501a30349a Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 23 Jun 2026 09:35:23 +0200 Subject: [PATCH 6/7] Revert "chore: replace Sender with Arc in Executor" This reverts commit f180e97859efd161b84d861b1a61908fa055a827. --- src/lib.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 7426740..ac7a273 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -304,9 +304,7 @@ impl Watch { } } - let mut exec = Executor::new(Arc::new(move || { - let _ = tx.send(WatchEvent::BuildFinished); - })); + let mut exec = Executor::new(tx.clone()); let mut lock_guard = Some(self.watch_lock.write()); // `pending_build` tracks whether a change has arrived that has not yet @@ -668,22 +666,22 @@ impl SharedChild { struct Executor { child: SharedChild, build_handle: Option>, - notify: Arc, + tx: mpsc::Sender, } impl Executor { - fn new(notify: Arc) -> Self { + fn new(tx: mpsc::Sender) -> Self { Self { child: SharedChild::new(), build_handle: None, - notify, + tx, } } /// Spawn a build on a background thread. fn spawn(&mut self, mut commands: CommandList) { let mut child = self.child.clone(); - let notify = self.notify.clone(); + let tx = self.tx.clone(); self.build_handle = Some(thread::spawn(move || { let mut status = ExitStatus::default(); @@ -709,7 +707,7 @@ impl Executor { log::error!("Command failed."); } - notify(); + let _ = tx.send(WatchEvent::BuildFinished); status.success() })); } From a696e84abde4debc82ce3e06877e2f1a103e0d8c Mon Sep 17 00:00:00 2001 From: Cecile Tonglet Date: Tue, 23 Jun 2026 10:47:02 +0200 Subject: [PATCH 7/7] chore: remove dead take_result poll from Timeout arm, fix doc --- src/lib.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ac7a273..e563b55 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -355,14 +355,6 @@ impl Watch { } } Err(mpsc::RecvTimeoutError::Timeout) => { - // Poll for build completion that finished during the wait. - if let Some(succeeded) = exec.take_result() { - if succeeded { - log::trace!("Command succeeded, releasing lock"); - lock_guard.take(); - } - } - // Quiet for `debounce` — time to build if there is a // pending change. if pending_build { @@ -661,7 +653,7 @@ impl SharedChild { /// Encapsulates a build running on a background thread. /// -/// Tracks the child process, generation counter, and [`JoinHandle`] so the +/// Tracks the child process and [`JoinHandle`] so the /// main loop can poll for completion without a dedicated channel. struct Executor { child: SharedChild,