mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-22 00:02:23 +00:00
fix(watch): stop a filesystem watch feeding itself on Linux (#523)
With a repository open, an idle window ran `git status -uall` about 2.6 times a second, forever. Measured on the Linux CI runner: 78 debounce bursts in 30 seconds, from one real change. `notify`'s inotify backend subscribes with `WatchMask::OPEN`, so every `open(2)` under a watched directory is an event. `git status` opens `.git/index`, `.git/HEAD` and `refs/heads/*`, all of which the source control watch covers — so the read that answers "did this repository change" is itself an event saying it may have changed, and the answer schedules the next question. The debounce caps the rate; it cannot break the cycle. `Debounce`'s own doc argued this was structurally impossible, because `GIT_OPTIONAL_LOCKS=0` stops `git status` writing the index back. That covers writes. `IN_OPEN` fires on reads, and no environment variable suppresses it. macOS FSEvents has no equivalent, so this was invisible on the machine it was written on, and every theory that did not involve the kernel was correctly eliminated before this one was found. `is_content_change` drops `Access(Open | Read | Close(Read))` and keeps `Access(Close(Write))`, which is a write finishing rather than a reader. Applied at both watchers: the host watch, and the config hot-reload watch in `main.rs`, which has the same shape — a reload re-reads the file it is watching. The guard is a host conformance case rather than a platform test, because macOS passes it trivially and Linux is where it has to hold. It was verified red on the Linux runner with the filter removed. Its drain waits out the create of the file it asserts about: FSEvents hands that over a beat after the stream opens, and a stale create is indistinguishable here from a read reporting.
This commit is contained in:
@@ -124,6 +124,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
### Fixed
|
||||
|
||||
- **An idle window stops re-reading the repository on Linux** — with a
|
||||
repository open, tty7 ran `git status -uall` about two and a half times a
|
||||
second, forever, with nothing on screen changing and nobody touching the
|
||||
machine. The filesystem watch was feeding itself: Linux reports opening a
|
||||
file as a watch event, every read of `.git` is an open, and reading `.git`
|
||||
is exactly how the watch's own events were answered — so each answer
|
||||
scheduled the next question. Reads are no longer mistaken for changes;
|
||||
a write finishing still is. macOS and Windows were never affected, which is
|
||||
why this survived so long.
|
||||
|
||||
- **An alias put back in an `Include`d ssh config file is found again** — the
|
||||
check behind a parked remote workspace watched `~/.ssh/config` for changes
|
||||
and nothing else, but `Include` is common and editing an included file
|
||||
|
||||
@@ -66,6 +66,7 @@ macro_rules! for_each_host_case {
|
||||
search_respects_max_dirs,
|
||||
shells_are_named_and_have_a_default,
|
||||
watch_reports_create_and_delete,
|
||||
watch_ignores_reads,
|
||||
watch_is_non_recursive,
|
||||
watch_set_dirs_adds_and_drops,
|
||||
watch_coalesces_within_window,
|
||||
@@ -946,6 +947,47 @@ pub fn watch_reports_create_and_delete(h: &dyn Host, sb: &dyn Sandbox) {
|
||||
);
|
||||
}
|
||||
|
||||
/// Reading a watched file is not a change.
|
||||
///
|
||||
/// The one case where a watch can feed itself: every consumer answers an event
|
||||
/// by reading the directory it came from, so if a read is an event, the answer
|
||||
/// asks the question again. `notify`'s inotify backend subscribes to `IN_OPEN`,
|
||||
/// which makes this reachable on Linux and only there — an idle Source Control
|
||||
/// panel ran `git status -uall` about 2.6 times a second before this was
|
||||
/// filtered (issue #523). macOS passes this trivially, which is exactly why it
|
||||
/// has to be a conformance case rather than a platform test.
|
||||
pub fn watch_ignores_reads(h: &dyn Host, sb: &dyn Sandbox) {
|
||||
let sandbox = sb.path();
|
||||
let f = h.join(sandbox, "read-me.txt");
|
||||
write(h, &f, "hello");
|
||||
|
||||
let sub = h.watch(&[sandbox.to_path_buf()]).unwrap();
|
||||
// The file this asserts about is created *before* the watch, so the drain
|
||||
// has to come after the tail of that create can still arrive — FSEvents
|
||||
// will hand one over a beat late, and a stale create is indistinguishable
|
||||
// here from the reads below reporting.
|
||||
std::thread::sleep(WATCH_QUIET);
|
||||
drain(&sub);
|
||||
|
||||
for _ in 0..5 {
|
||||
h.read_file(&f, 1 << 20)
|
||||
.expect("the watched file reads back");
|
||||
}
|
||||
std::thread::sleep(WATCH_QUIET);
|
||||
|
||||
let batches = collect_batches(&sub, Duration::from_millis(200));
|
||||
let leaked: Vec<&PathBuf> = batches
|
||||
.iter()
|
||||
.flatten()
|
||||
.filter(|p| p.file_name().is_some_and(|n| n == "read-me.txt"))
|
||||
.collect();
|
||||
assert!(
|
||||
leaked.is_empty(),
|
||||
"reading a watched file reported as a change, so a watch can feed \
|
||||
itself: {leaked:?}"
|
||||
);
|
||||
}
|
||||
|
||||
pub fn watch_is_non_recursive(h: &dyn Host, sb: &dyn Sandbox) {
|
||||
let sandbox = sb.path();
|
||||
let sub_dir = h.join(sandbox, "child");
|
||||
|
||||
@@ -381,6 +381,7 @@ fn local_watch(dirs: &[PathBuf], gitignore: Arc<Mutex<GitignoreChain>>) -> io::R
|
||||
let watcher = notify::recommended_watcher(move |res: notify::Result<notify::Event>| {
|
||||
if let Ok(ev) = res
|
||||
&& !ev.paths.is_empty()
|
||||
&& crate::host::is_content_change(&ev.kind)
|
||||
{
|
||||
let _ = raw_tx.send(ev.paths);
|
||||
}
|
||||
|
||||
@@ -262,6 +262,32 @@ pub trait Host: Send + Sync + 'static {
|
||||
}
|
||||
}
|
||||
|
||||
/// Did this watch event mean something *changed*, or only that something was
|
||||
/// read?
|
||||
///
|
||||
/// The distinction does not exist on macOS, and on Linux it decides whether a
|
||||
/// watch can feed itself. `notify`'s inotify backend asks for `IN_OPEN`, so
|
||||
/// every `open(2)` under a watched directory arrives as an event — and the one
|
||||
/// thing every consumer of a watch does with an event is go and read the
|
||||
/// directory it came from. Reading `.git/index` to answer "did the repository
|
||||
/// change" is itself an event saying the repository may have changed, so the
|
||||
/// answer schedules the next question, at whatever rate the debounce allows,
|
||||
/// forever. Measured at ~2.6 `git status -uall` per second on an idle Source
|
||||
/// Control panel before this filter existed (issue #523).
|
||||
///
|
||||
/// `IN_CLOSE_WRITE` also arrives as `Access`, and that one is a real write
|
||||
/// finishing, so it stays. Everything else under `Access` is a reader.
|
||||
pub fn is_content_change(kind: ¬ify::EventKind) -> bool {
|
||||
use notify::event::{AccessKind, AccessMode};
|
||||
|
||||
!matches!(
|
||||
kind,
|
||||
notify::EventKind::Access(
|
||||
AccessKind::Open(_) | AccessKind::Read | AccessKind::Close(AccessMode::Read),
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
pub fn default_join(dir: &Path, name: &str, sep: char) -> PathBuf {
|
||||
let mut s = dir.to_string_lossy().into_owned();
|
||||
if !s.is_empty() && !s.ends_with(sep) && !s.ends_with('/') {
|
||||
|
||||
@@ -43,6 +43,11 @@ fn spawn_config_watcher(cx: &mut App) {
|
||||
let watched_file = config_file.clone();
|
||||
let handler = move |res: notify::Result<notify::Event>| {
|
||||
let Ok(event) = res else { return };
|
||||
// A reload re-reads the file, and on Linux reading it is itself an
|
||||
// event — see `tty7_core::host::is_content_change`.
|
||||
if !tty7_core::host::is_content_change(&event.kind) {
|
||||
return;
|
||||
}
|
||||
let hit = event
|
||||
.paths
|
||||
.iter()
|
||||
|
||||
Reference in New Issue
Block a user