Skip to content

Commit 8e8e3f0

Browse files
committed
fix(resync): close watch and carrier gaps
1 parent b30015e commit 8e8e3f0

2 files changed

Lines changed: 114 additions & 14 deletions

File tree

docs/vrs/06-resync/spec.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,9 @@ Resource projection so absence of coverage is observable, never silent.
5252

5353
Windows are constants in one place, named as provisional, and tuned by
5454
observed notification volume (issue #341 rollout note). A burst inside a
55-
window collapses to one pass; within that pass each changed carrier still
56-
gets its own event so per-binding supersession stays meaningful.
55+
window collapses to one pass; when subscribers of different classes share a
56+
path, each remains dirty until its own class window. Within one due class each
57+
changed carrier gets its own event so per-binding supersession stays meaningful.
5758

5859
## Watcher mechanics
5960

@@ -71,11 +72,13 @@ gets its own event so per-binding supersession stays meaningful.
7172
bus id, label, and class so every subscriber retains its digest and pending
7273
dirty state even when multiple bindings share one path; only new
7374
subscriptions seed silently.
74-
- A previously blind path is digest-diffed before its recovered parent directory is added to the
75-
registered-watch set, covering changes made during the blind interval.
75+
- A previously blind path is digest-diffed both before and after its recovered
76+
parent watch is registered, closing the poll-to-registration gap.
7677
- Installation failure is diagnosed once and degrades to timer-based digest
7778
polling over the watch set (bounded by the number of bindings), never to
7879
silence about the mechanism.
80+
- Digest reads open carriers nonblocking and accept regular files only; FIFOs and other special
81+
files cannot stall the worker.
7982

8083
## Built-in stream
8184

src/resync.rs

Lines changed: 107 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,10 @@ impl Worker {
328328
// Diff paths that were blind before registering newly recovered parents; otherwise the
329329
// new watch suppresses polling of mutations that happened during the blind interval.
330330
self.poll_unwatched();
331-
self.refresh_watches();
331+
let registered = self.refresh_watches();
332+
// Registration closes the event gap first; this second digest pass covers writes between
333+
// the pre-registration poll and watch installation.
334+
self.poll_registered(&registered);
332335
}
333336

334337
/// Digest-diff every entry in `next`, emitting observed transitions. Seeding stays silent
@@ -377,15 +380,31 @@ impl Worker {
377380
.cloned()
378381
.collect();
379382
for path in unwatched {
380-
self.flush_path(&path);
383+
self.flush_path(&path, None);
384+
}
385+
}
386+
387+
fn poll_registered(&mut self, directories: &[PathBuf]) {
388+
let paths: Vec<PathBuf> = self
389+
.carriers
390+
.keys()
391+
.filter(|path| {
392+
path.parent()
393+
.is_some_and(|parent| directories.iter().any(|dir| dir == parent))
394+
})
395+
.cloned()
396+
.collect();
397+
for path in paths {
398+
self.flush_path(&path, None);
381399
}
382400
}
383401

384402
/// Re-register watches so every distinct parent directory of the current watch set is covered,
385403
/// dropping directories that left the set or were replaced (identity change). A replaced watch
386404
/// stays blind until the next pass rebuilds it — bounded by the reconcile interval, the same
387405
/// tradeoff `CatalogDeclarationWatcher` accepts for declarations.
388-
fn refresh_watches(&mut self) {
406+
fn refresh_watches(&mut self) -> Vec<PathBuf> {
407+
let mut registered = Vec::new();
389408
let mut desired: Vec<PathBuf> = Vec::new();
390409
for path in self.carriers.keys() {
391410
if let Some(parent) = path.parent() {
@@ -418,9 +437,11 @@ impl Worker {
418437
.watch(&dir, notify::RecursiveMode::NonRecursive)
419438
.is_ok()
420439
{
440+
registered.push(dir.clone());
421441
self.watched.insert(dir, identity);
422442
}
423443
}
444+
registered
424445
}
425446

426447
fn mark_mutated(&mut self, paths: Vec<PathBuf>) {
@@ -459,7 +480,8 @@ impl Worker {
459480
drop(dirty_here);
460481
}
461482
if extend {
462-
self.refresh_watches();
483+
let registered = self.refresh_watches();
484+
self.poll_registered(&registered);
463485
}
464486
}
465487

@@ -483,19 +505,23 @@ impl Worker {
483505
.map(|(path, _)| path.clone())
484506
.collect();
485507
for path in targets {
486-
self.flush_path(&path);
508+
self.flush_path(&path, Some(class));
487509
}
488510
}
489511
}
490512

491-
/// Flush every subscriber of one path: diff digests, emit transitions, and retain failed
492-
/// publications for retry with the same event identity.
493-
fn flush_path(&mut self, path: &Path) {
513+
/// Flush subscribers of one path whose class is due, or every subscriber for fallback polls:
514+
/// diff digests, emit transitions, and retain failed publications for retry with the same
515+
/// event identity.
516+
fn flush_path(&mut self, path: &Path, due_class: Option<CarrierClass>) {
494517
let Some(entries) = self.carriers.get_mut(path) else {
495518
return;
496519
};
497520
let mut retries = Vec::new();
498521
for entry in entries.iter_mut() {
522+
if due_class.is_some_and(|class| entry.class != class) {
523+
continue;
524+
}
499525
entry.dirty = false;
500526
let Some(new_digest) = read_digest(path) else {
501527
// Unreadable right now (deleted mid-window): stay quiet and keep the previous
@@ -573,10 +599,36 @@ fn emit_resync(
573599
}
574600

575601
fn read_digest(path: &Path) -> Option<String> {
576-
let bytes = std::fs::read(path).ok()?;
602+
let bytes = read_regular(path)?;
577603
Some(format!("{:x}", Sha256::digest(&bytes)))
578604
}
579605

606+
#[cfg(unix)]
607+
fn read_regular(path: &Path) -> Option<Vec<u8>> {
608+
use std::io::Read as _;
609+
use std::os::unix::fs::OpenOptionsExt as _;
610+
611+
let mut file = std::fs::OpenOptions::new()
612+
.read(true)
613+
.custom_flags(libc::O_NOFOLLOW | libc::O_CLOEXEC | libc::O_NONBLOCK)
614+
.open(path)
615+
.ok()?;
616+
if !file.metadata().ok()?.file_type().is_file() {
617+
return None;
618+
}
619+
let mut bytes = Vec::new();
620+
file.read_to_end(&mut bytes).ok()?;
621+
Some(bytes)
622+
}
623+
624+
#[cfg(not(unix))]
625+
fn read_regular(path: &Path) -> Option<Vec<u8>> {
626+
if !std::fs::metadata(path).ok()?.file_type().is_file() {
627+
return None;
628+
}
629+
std::fs::read(path).ok()
630+
}
631+
580632
fn render_body(label: &str, path: &Path, old: Option<&str>, new: &str) -> String {
581633
format!(
582634
"resource `{label}` changed\n\nbinding: {label}\npath: {}\nold: {}\nnew: {new}\n",
@@ -694,6 +746,51 @@ mod tests {
694746
}
695747
}
696748

749+
#[cfg(unix)]
750+
#[test]
751+
fn digesting_a_fifo_fails_without_blocking_the_worker() {
752+
use std::os::unix::ffi::OsStrExt as _;
753+
754+
let tmp = tempfile::tempdir().unwrap();
755+
let fifo = tmp.path().join("carrier.fifo");
756+
let fifo_c = std::ffi::CString::new(fifo.as_os_str().as_bytes()).unwrap();
757+
// SAFETY: the path is NUL-terminated and points into the live temp directory.
758+
assert_eq!(unsafe { libc::mkfifo(fifo_c.as_ptr(), 0o600) }, 0);
759+
assert_eq!(read_digest(&fifo), None);
760+
}
761+
762+
#[test]
763+
fn due_flush_only_clears_subscribers_of_the_due_class() {
764+
let root = tempfile::tempdir().unwrap();
765+
let carrier = root.path().join("carrier.md");
766+
std::fs::write(&carrier, "same bytes").unwrap();
767+
let digest = read_digest(&carrier);
768+
let entries = [CarrierClass::Immediate, CarrierClass::Coalesced]
769+
.into_iter()
770+
.map(|class| Entry {
771+
bus_id: "host.worker".to_owned(),
772+
label: format!("{class:?}"),
773+
class,
774+
digest: digest.clone(),
775+
dirty: true,
776+
})
777+
.collect();
778+
let now = Instant::now();
779+
let mut worker = Worker {
780+
root: root.path().to_path_buf(),
781+
this_host: "host".to_owned(),
782+
carriers: BTreeMap::from([(carrier, entries)]),
783+
deadlines: BTreeMap::from([(CarrierClass::Immediate, now)]),
784+
watched: BTreeMap::new(),
785+
watcher: None,
786+
};
787+
788+
worker.flush_due(now);
789+
let entries = worker.carriers.values().next().unwrap();
790+
assert!(!entries[0].dirty);
791+
assert!(entries[1].dirty, "coalesced subscriber must wait for its own deadline");
792+
}
793+
697794
#[test]
698795
fn failed_emit_retains_digest_and_schedules_the_same_transition_for_retry() {
699796
let root = tempfile::tempdir().unwrap();
@@ -717,7 +814,7 @@ mod tests {
717814
watcher: None,
718815
};
719816

720-
worker.flush_path(&carrier);
817+
worker.flush_path(&carrier, None);
721818
let entry = &worker.carriers[&carrier][0];
722819
assert_eq!(entry.digest.as_deref(), Some("old-digest"));
723820
assert!(entry.dirty);

0 commit comments

Comments
 (0)