hakowork/rojo
3
0
Fork 0
mirror of https://github.com/rojo-rbx/rojo.git synced 2026-10-23 05:51:39 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Refactor file watcher and path suppression handling for improved reliability

Browse source 
- Updated the `StdBackend` to use a platform-specific watcher type, enhancing event delivery on macOS by utilizing `PollWatcher`.
- Refactored the path suppression logic in `JobThreadContext` to handle any VFS event type, improving the accuracy of event processing during renames and writes.
- Enhanced documentation to clarify the changes in event handling and suppression mechanisms, contributing to better maintainability and understanding of the code.
This commit is contained in:
Joe 2026-02-06 13:58:59 -06:00
parent 4ee1e79a90
commit 0605d5cfee
2 changed files with 109 additions and 53 deletions
Download patch file Download diff file Expand all files Collapse all files

24
crates/memofs/src/std_backend.rs
View file

@ -5,11 +5,19 @@ use std::{collections::HashSet, io};
use crossbeam_channel::{Receiver, Sender}; use crossbeam_channel::{Receiver, Sender};
use notify::RecursiveMode; use notify::RecursiveMode;
use notify_debouncer_full::{ use notify_debouncer_full::{
new_debouncer, new_debouncer_opt,
notify::event::{CreateKind, EventKind, ModifyKind, RemoveKind, RenameMode}, notify::event::{CreateKind, EventKind, ModifyKind, RemoveKind, RenameMode},
DebounceEventResult, Debouncer, RecommendedCache, DebounceEventResult, Debouncer, FileIdMap,
}; };
/// On macOS, FSEvents silently fails to deliver events in `/var/folders/`
/// temp directories (a known platform limitation). Use PollWatcher instead
/// for reliable event delivery on all directory types.
#[cfg(target_os = "macos")]
type PlatformWatcher = notify::PollWatcher;
#[cfg(not(target_os = "macos"))]
type PlatformWatcher = notify::RecommendedWatcher;
use crate::{DirEntry, Metadata, ReadDir, VfsBackend, VfsEvent}; use crate::{DirEntry, Metadata, ReadDir, VfsBackend, VfsEvent};
/// Critical errors from the file watcher that indicate watching is no longer reliable. /// Critical errors from the file watcher that indicate watching is no longer reliable.
@ -55,7 +63,7 @@ pub type CriticalErrorHandler = Box<dyn Fn(WatcherCriticalError) -> bool + Send
/// `VfsBackend` that uses `std::fs` and the `notify` crate. /// `VfsBackend` that uses `std::fs` and the `notify` crate.
pub struct StdBackend { pub struct StdBackend {
debouncer: Debouncer<notify::RecommendedWatcher, RecommendedCache>, debouncer: Debouncer<PlatformWatcher, FileIdMap>,
watcher_receiver: Receiver<VfsEvent>, watcher_receiver: Receiver<VfsEvent>,
watches: HashSet<PathBuf>, watches: HashSet<PathBuf>,
/// Receiver for critical errors from the watcher thread. /// Receiver for critical errors from the watcher thread.
@ -98,11 +106,15 @@ impl StdBackend {
event_tx: Sender<VfsEvent>, event_tx: Sender<VfsEvent>,
error_tx: Sender<WatcherCriticalError>, error_tx: Sender<WatcherCriticalError>,
error_handler: CriticalErrorHandler, error_handler: CriticalErrorHandler,
) -> Debouncer<notify::RecommendedWatcher, RecommendedCache> { ) -> Debouncer<PlatformWatcher, FileIdMap> {
// Use 50ms debounce timeout (same as the old v4 implementation) // Use 50ms debounce timeout (same as the old v4 implementation)
let debounce_timeout = Duration::from_millis(50); let debounce_timeout = Duration::from_millis(50);
new_debouncer( // On macOS we use PollWatcher which needs an explicit poll interval.
// On other platforms this field is ignored by RecommendedWatcher.
let config = notify::Config::default().with_poll_interval(debounce_timeout);
new_debouncer_opt::<_, PlatformWatcher, FileIdMap>(
debounce_timeout, debounce_timeout,
None, // Use default tick rate None, // Use default tick rate
move |result: DebounceEventResult| { move |result: DebounceEventResult| {
@ -145,6 +157,8 @@ impl StdBackend {
} }
} }
}, },
FileIdMap::new(),
config,
) )
.expect("Failed to create file watcher debouncer") .expect("Failed to create file watcher debouncer")
} }

128
src/change_processor.rs
View file

@ -187,13 +187,6 @@ impl JobThreadContext {
suppressed.entry(key).or_insert((0, 0)).1 += 1; suppressed.entry(key).or_insert((0, 0)).1 += 1;
} }
/// Suppress the next Remove VFS event for the given path.
fn suppress_path_remove(&self, path: &Path) {
let mut suppressed = self.suppressed_paths.lock().unwrap();
let key = Self::suppression_key(path);
suppressed.entry(key).or_insert((0, 0)).0 += 1;
}
/// Remove a Create/Write suppression previously added by [`suppress_path`]. /// Remove a Create/Write suppression previously added by [`suppress_path`].
/// Called when the filesystem operation failed, so that future VFS events /// Called when the filesystem operation failed, so that future VFS events
/// for that path are not incorrectly swallowed. /// for that path are not incorrectly swallowed.
@ -208,12 +201,25 @@ impl JobThreadContext {
} }
} }
/// Remove a Remove suppression previously added by [`suppress_path_remove`]. /// Suppress the next VFS event of ANY type for the given path.
fn unsuppress_path_remove(&self, path: &Path) { /// Used for the **old** path of renames: different platforms deliver
/// different event types for the source of a rename (REMOVE on
/// Linux/Windows, stale CREATE on macOS FSEvents).
fn suppress_path_any(&self, path: &Path) {
let mut suppressed = self.suppressed_paths.lock().unwrap();
let key = Self::suppression_key(path);
let entry = suppressed.entry(key).or_insert((0, 0));
entry.0 += 1;
entry.1 += 1;
}
/// Remove both counters previously added by [`suppress_path_any`].
fn unsuppress_path_any(&self, path: &Path) {
let mut suppressed = self.suppressed_paths.lock().unwrap(); let mut suppressed = self.suppressed_paths.lock().unwrap();
let key = Self::suppression_key(path); let key = Self::suppression_key(path);
if let Some(counts) = suppressed.get_mut(&key) { if let Some(counts) = suppressed.get_mut(&key) {
counts.0 = counts.0.saturating_sub(1); counts.0 = counts.0.saturating_sub(1);
counts.1 = counts.1.saturating_sub(1);
if counts.0 == 0 && counts.1 == 0 { if counts.0 == 0 && counts.1 == 0 {
suppressed.remove(&key); suppressed.remove(&key);
} }
@ -361,10 +367,6 @@ impl JobThreadContext {
// of the tree. Calculate and apply all of these changes. // of the tree. Calculate and apply all of these changes.
let applied_patches = match event { let applied_patches = match event {
VfsEvent::Create(path) | VfsEvent::Write(path) => { VfsEvent::Create(path) | VfsEvent::Write(path) => {
// The path might have been deleted before we could process this event
// (e.g., during syncback when directories are being removed).
// If canonicalize fails, retry once after a brief delay — on Windows,
// the file may still be locked by the editor process.
match self.vfs.canonicalize(&path) { match self.vfs.canonicalize(&path) {
Ok(canonical_path) => { Ok(canonical_path) => {
log::info!( log::info!(
@ -374,35 +376,77 @@ impl JobThreadContext {
); );
self.apply_patches(canonical_path) self.apply_patches(canonical_path)
} }
Err(_) => {
// The path doesn't exist on disk. Two possible causes:
//
// 1. Phantom event from a rename we performed — macOS FSEvents
// can deliver a stale CREATE for the old path. If a pending
// suppression exists for this path, consume it and skip.
//
// 2. The file was deleted between the event firing and us
// processing it. Fall back to parent-directory canonicalize
// (same strategy as the Remove handler) so the tree can
// reconcile the disappearance.
let consumed = {
let key = Self::suppression_key(&path);
let mut suppressed = self.suppressed_paths.lock().unwrap();
if let Some(counts) = suppressed.get_mut(&key) {
if counts.0 > 0 || counts.1 > 0 {
// Drain whichever counter is available
if counts.1 > 0 {
counts.1 -= 1;
} else {
counts.0 -= 1;
}
if counts.0 == 0 && counts.1 == 0 {
suppressed.remove(&key);
}
true
} else {
false
}
} else {
false
}
};
if consumed {
log::info!(
"VFS: phantom Create/Write for non-existent {} — \
consumed pending suppression (likely stale rename event)",
path.display()
);
Vec::new()
} else {
// No suppression — the file genuinely vanished. Use the
// parent-directory fallback so the tree can reconcile.
let parent = path.parent().unwrap();
let file_name = path.file_name().unwrap();
match self.vfs.canonicalize(parent) {
Ok(parent_normalized) => {
let resolved = parent_normalized.join(file_name);
log::info!(
"VFS: Create/Write for vanished file {} — \
resolved via parent to {}",
path.display(),
resolved.display()
);
self.apply_patches(resolved)
}
Err(err) => { Err(err) => {
log::info!( log::info!(
"VFS: canonicalize FAILED for {} ({}), retrying after 50ms...", "VFS: Skipping Create/Write for {} — \
parent no longer exists: {}",
path.display(), path.display(),
err err
); );
std::thread::sleep(Duration::from_millis(50));
match self.vfs.canonicalize(&path) {
Ok(canonical_path) => {
log::info!(
"VFS: retry canonicalize OK for {} -> {}",
path.display(),
canonical_path.display()
);
self.apply_patches(canonical_path)
}
Err(err2) => {
log::warn!(
"VFS: Create/Write event DROPPED for {} after retry — \
canonicalize failed: {}. File may be desynchronized!",
path.display(),
err2
);
Vec::new() Vec::new()
} }
} }
} }
} }
} }
}
VfsEvent::Remove(path) => { VfsEvent::Remove(path) => {
// MemoFS does not track parent removals yet, so we can canonicalize // MemoFS does not track parent removals yet, so we can canonicalize
// the parent path safely and then append the removed path's file name. // the parent path safely and then append the removed path's file name.
@ -627,12 +671,12 @@ impl JobThreadContext {
dir_path.display(), dir_path.display(),
new_dir_path.display() new_dir_path.display()
); );
self.suppress_path_remove(dir_path); self.suppress_path_any(dir_path);
self.suppress_path(&new_dir_path); self.suppress_path(&new_dir_path);
if let Err(err) = if let Err(err) =
fs::rename(dir_path, &new_dir_path) fs::rename(dir_path, &new_dir_path)
{ {
self.unsuppress_path_remove(dir_path); self.unsuppress_path_any(dir_path);
self.unsuppress_path(&new_dir_path); self.unsuppress_path(&new_dir_path);
log::error!( log::error!(
"Failed to rename directory {:?} to {:?}: {}", "Failed to rename directory {:?} to {:?}: {}",
@ -654,14 +698,12 @@ impl JobThreadContext {
"{}.meta.json5", "{}.meta.json5",
encoded_new_name encoded_new_name
)); ));
self.suppress_path_remove(&old_meta); self.suppress_path_any(&old_meta);
self.suppress_path(&new_meta); self.suppress_path(&new_meta);
if fs::rename(&old_meta, &new_meta) if fs::rename(&old_meta, &new_meta)
.is_err() .is_err()
{ {
self.unsuppress_path_remove( self.unsuppress_path_any(&old_meta);
&old_meta,
);
self.unsuppress_path(&new_meta); self.unsuppress_path(&new_meta);
} }
} }
@ -713,10 +755,10 @@ impl JobThreadContext {
path.display(), path.display(),
new_path.display() new_path.display()
); );
self.suppress_path_remove(path); self.suppress_path_any(path);
self.suppress_path(&new_path); self.suppress_path(&new_path);
if let Err(err) = fs::rename(path, &new_path) { if let Err(err) = fs::rename(path, &new_path) {
self.unsuppress_path_remove(path); self.unsuppress_path_any(path);
self.unsuppress_path(&new_path); self.unsuppress_path(&new_path);
log::error!( log::error!(
"Failed to rename {:?} to {:?}: {}", "Failed to rename {:?} to {:?}: {}",
@ -733,11 +775,11 @@ impl JobThreadContext {
"{}.meta.json5", "{}.meta.json5",
encoded_new_name encoded_new_name
)); ));
self.suppress_path_remove(&old_meta); self.suppress_path_any(&old_meta);
self.suppress_path(&new_meta); self.suppress_path(&new_meta);
if fs::rename(&old_meta, &new_meta).is_err() if fs::rename(&old_meta, &new_meta).is_err()
{ {
self.unsuppress_path_remove(&old_meta); self.unsuppress_path_any(&old_meta);
self.unsuppress_path(&new_meta); self.unsuppress_path(&new_meta);
} }
} }
@ -852,12 +894,12 @@ impl JobThreadContext {
actual_file.display(), actual_file.display(),
new_path.display() new_path.display()
); );
self.suppress_path_remove(&actual_file); self.suppress_path_any(&actual_file);
self.suppress_path(&new_path); self.suppress_path(&new_path);
if let Err(err) = if let Err(err) =
fs::rename(&actual_file, &new_path) fs::rename(&actual_file, &new_path)
{ {
self.unsuppress_path_remove(&actual_file); self.unsuppress_path_any(&actual_file);
self.unsuppress_path(&new_path); self.unsuppress_path(&new_path);
log::error!( log::error!(
"Failed to rename {:?} to {:?}: {}", "Failed to rename {:?} to {:?}: {}",