fix(vcs): break the .git/index self-refresh loop that froze the UI (#339)

With a project open, the app became slow and then unresponsive after 30-60 minutes. Git status writes refreshed stat data back to .git/index; the watcher reported that write as a file change and a VCS change; the UI scheduled another status ~250 ms later, which wrote the index again. With any stat-dirty file this never settled: a full status, IPC traffic and decoration rebuilds in an endless loop.

Watcher batches now carry .git internals separately (FsBatch.vcs), so they no longer appear as worktree changes and no fsChanged is sent for .git-only batches. Status records the identity (size, mtime) of the index it writes, and the host skips VcsChanged only for that write; external index changes (git add / reset / stash from a terminal) and HEAD / refs / packed-refs changes still refresh as before. The UI's fsChanged handler also ignores .git-only batches.

Fixes #311

Co-authored-by: a-rookie-of-C-language <186315048+a-rookie-of-C-language@users.noreply.github.com>
This commit is contained in:
arookieofc
2026-10-30 13:34:18 +08:00
committed by GitHub
co-authored by a-rookie-of-C-language
parent 0a27d02f4d
commit ab6fca01c4
8 changed files with 163 additions and 20 deletions
+51 -2
View File
@@ -234,7 +234,7 @@ impl Host {
// A repository was cloned / initialised / removed below the project: its
// `.git/HEAD` (the watcher reports nothing else of `.git`) appeared or went.
let known = repos.read().roots();
let git_dirs = changed.iter().chain(&batch.removed).any(|p| {
let git_dirs = changed.iter().chain(&batch.removed).chain(&batch.vcs).any(|p| {
let Some(dir) = p.ends_with(".git/HEAD").then(|| p.parent().and_then(Path::parent)).flatten()
else {
return false;
@@ -244,7 +244,10 @@ impl Host {
if git_dirs {
scan_repositories(&root, &repos, &settings, &sink);
}
sink.emit(IdeEvent::VcsChanged { root: root.clone().into() });
let vcs_moved = batch.vcs.iter().any(|p| vcs_path_changed(p));
if git_dirs || vcs_moved {
sink.emit(IdeEvent::VcsChanged { root: root.clone().into() });
}
}),
)?
};
@@ -1008,3 +1011,49 @@ mod maven_linkage_tests {
assert!(model.maven.unwrap().projects.is_empty());
}
}
fn vcs_path_changed(path: &Path) -> bool {
!path.ends_with(".git/index") || !ide_vcs::is_own_index_write(path)
}
#[cfg(test)]
mod index_event_tests {
use super::vcs_path_changed;
#[test]
fn status_write_is_suppressed_but_external_index_and_refs_refresh() {
let tmp = tempfile::tempdir().unwrap();
let dir = tmp.path();
let git = |args: &[&str]| {
let output = std::process::Command::new("git").current_dir(dir).args(args).output().unwrap();
assert!(output.status.success(), "{}", String::from_utf8_lossy(&output.stderr));
};
git(&["init", "-q"]);
let file = dir.join("file.txt");
std::fs::write(&file, "original\n").unwrap();
git(&["add", "file.txt"]);
// Same contents, different stat data: status must refresh the index.
std::fs::File::options()
.write(true)
.open(&file)
.unwrap()
.set_modified(std::time::SystemTime::UNIX_EPOCH + std::time::Duration::from_secs(1))
.unwrap();
let index = dir.join(".git/index");
let before = std::fs::read(&index).unwrap();
let repo = ide_vcs::Repo::discover(dir).unwrap().unwrap();
repo.status().unwrap();
assert_ne!(before, std::fs::read(&index).unwrap(), "status must actually write the index");
assert!(!vcs_path_changed(&index));
assert!(!vcs_path_changed(&index), "duplicate notifications for the same identity");
for path in [".git/HEAD", ".git/refs/heads/main", ".git/packed-refs"] {
assert!(vcs_path_changed(&dir.join(path)));
}
std::fs::write(&file, "external staged change\n").unwrap();
git(&["add", "file.txt"]);
assert!(vcs_path_changed(&index));
assert!(vcs_path_changed(&index));
std::fs::remove_file(&index).unwrap();
assert!(vcs_path_changed(&index));
}
}
+77
View File
@@ -0,0 +1,77 @@
//! Identities of index files last written by status, shared with filesystem watchers.
use std::collections::HashMap;
use std::path::{Path, PathBuf};
use std::sync::{Mutex, MutexGuard, OnceLock};
use std::time::SystemTime;
#[derive(PartialEq, Eq)]
struct Identity {
len: u64,
modified: SystemTime,
}
fn identity(path: &Path) -> Option<Identity> {
let metadata = path.metadata().ok()?;
Some(Identity { len: metadata.len(), modified: metadata.modified().ok()? })
}
#[derive(Default)]
pub(crate) struct IndexWrites(HashMap<PathBuf, Identity>);
pub(crate) fn lock() -> MutexGuard<'static, IndexWrites> {
static WRITES: OnceLock<Mutex<IndexWrites>> = OnceLock::new();
WRITES.get_or_init(Mutex::default).lock().unwrap_or_else(|p| p.into_inner())
}
impl IndexWrites {
pub(crate) fn record(&mut self, path: &Path) {
let Ok(path) = path.canonicalize() else { return };
// Bound memory even when many repositories are opened over the process lifetime.
if self.0.len() >= 256 && !self.0.contains_key(&path) {
self.0.clear();
}
if let Some(identity) = identity(&path) {
self.0.insert(path, identity);
}
}
}
/// Whether the current index still matches our last status write-back. Retain a
/// match for duplicate watcher notifications; invalidate it on an external write.
pub fn is_own_index_write(path: &Path) -> bool {
let mut writes = lock();
let Ok(path) = path.canonicalize() else { return false };
if let Some(current) = identity(&path)
&& writes.0.get(&path) == Some(&current)
{
return true;
}
writes.0.remove(&path);
false
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn identities_are_scoped_to_paths_and_invalidated_by_metadata_changes() {
let tmp = tempfile::tempdir().unwrap();
let first = tmp.path().join("index");
let second = tmp.path().join("other-index");
std::fs::write(&first, "first").unwrap();
std::fs::write(&second, "first").unwrap();
lock().record(&first);
assert!(is_own_index_write(&first));
assert!(!is_own_index_write(&second));
let file = std::fs::File::options().write(true).open(&first).unwrap();
file.set_modified(SystemTime::UNIX_EPOCH).unwrap();
assert!(!is_own_index_write(&first));
lock().record(&first);
file.set_len(10).unwrap();
assert!(!is_own_index_write(&first));
std::fs::remove_file(&first).unwrap();
assert!(!is_own_index_write(&first));
}
}
+2
View File
@@ -443,6 +443,7 @@ mod cli;
mod diff;
mod history;
mod ignore;
mod index_writes;
mod manage;
mod ops;
mod patch;
@@ -452,6 +453,7 @@ mod repo;
mod roots;
pub mod shelf;
mod status;
pub use index_writes::is_own_index_write;
mod text_encoding;
mod write;
+7 -5
View File
@@ -90,14 +90,16 @@ pub(crate) fn status(r: &Repo) -> IdeResult<Vec<FileStatus>> {
}
}
}
// Persist refreshed stat info so the next status is cheaper (like `git status` does).
// Only when something changed: `write_changes` writes even an empty change set, and
// the rewritten `.git/index` comes back as a VCS change that triggers the next status.
// Serialize write-back and identity capture with watcher queries.
if let Some(mut outcome) = iter.into_outcome()
&& outcome.has_changes()
&& let Some(Err(e)) = outcome.write_changes()
{
tracing::debug!("could not write refreshed index: {e}");
let mut writes = crate::index_writes::lock();
match outcome.write_changes() {
Some(Ok(_)) => writes.record(&repo.index_path()),
Some(Err(e)) => tracing::debug!("could not write refreshed index: {e}"),
None => {}
}
}
Ok(map
+6 -1
View File
@@ -305,11 +305,16 @@ pub struct FsBatch {
pub created: Vec<PathBuf>,
pub modified: Vec<PathBuf>,
pub removed: Vec<PathBuf>,
/// VCS metadata (`.git/HEAD`, `index`, `refs/**`): reported separately so
/// generic `fsChanged` consumers do not treat the backend's own index
/// refresh as a worktree edit and schedule another `vcs_status` (which
/// writes `.git/index` again — an idle refresh loop).
pub vcs: Vec<PathBuf>,
}
impl FsBatch {
pub fn is_empty(&self) -> bool {
self.created.is_empty() && self.modified.is_empty() && self.removed.is_empty()
self.created.is_empty() && self.modified.is_empty() && self.removed.is_empty() && self.vcs.is_empty()
}
}
+13 -10
View File
@@ -77,11 +77,16 @@ impl Watcher {
let shared = filter.excludes.clone();
let rules = filter.gitignore.clone();
let emit = move |batch: &FsBatch| {
sink.emit(IdeEvent::FsChanged {
created: batch.created.iter().map(Into::into).collect(),
modified: batch.modified.iter().map(Into::into).collect(),
removed: batch.removed.iter().map(Into::into).collect(),
});
// VCS-metadata-only batches (`batch.vcs`) carry no worktree paths:
// consumers that refresh on any `fsChanged` would reschedule purely
// from the backend's own `.git/index` write.
if !(batch.created.is_empty() && batch.modified.is_empty() && batch.removed.is_empty()) {
sink.emit(IdeEvent::FsChanged {
created: batch.created.iter().map(Into::into).collect(),
modified: batch.modified.iter().map(Into::into).collect(),
removed: batch.removed.iter().map(Into::into).collect(),
});
}
on_change(batch);
};
let inner = match mode {
@@ -517,7 +522,7 @@ impl EventFilter {
let mut batch = FsBatch::default();
for (path, change) in changes {
if change == Change::Vcs {
batch.modified.push(path);
batch.vcs.push(path);
continue;
}
let meta = path.symlink_metadata().ok();
@@ -594,10 +599,8 @@ mod tests {
];
let batch = filter.batch(&events);
assert_eq!(batch.created, vec![root.join("a.txt")]);
assert_eq!(
batch.modified,
vec![root.join(".git/HEAD"), root.join(".git/refs/heads/feature"), root.join("kept.txt")]
);
assert_eq!(batch.vcs, vec![root.join(".git/HEAD"), root.join(".git/refs/heads/feature")]);
assert_eq!(batch.modified, vec![root.join("kept.txt")]);
assert_eq!(batch.removed, vec![root.join("from.txt"), root.join("gone.txt"), root.join("old.txt")]);
}
+1 -1
View File
@@ -297,7 +297,7 @@ fn watcher_reports_changes() {
fs::remove_file(r.join("existing.txt")).unwrap();
let all = |b: &[FsBatch]| -> BTreeSet<PathBuf> {
b.iter().flat_map(|b| b.created.iter().chain(&b.modified).chain(&b.removed).cloned()).collect()
b.iter().flat_map(|b| b.created.iter().chain(&b.modified).chain(&b.removed).chain(&b.vcs).cloned()).collect()
};
assert!(
wait_for(&batches, |b| {
+6 -1
View File
@@ -275,7 +275,12 @@ export function installVcsStore(): () => void {
if (useVcs.getState().available) scheduleRefresh(200);
else void discoverRoots();
});
const offFs = onIdeEvent("fsChanged", () => scheduleRefresh(250));
// `.git/**` internals show up through other watchers too; refreshing status
// on those alone rewrites `.git/index` and re-triggers this handler.
const offFs = onIdeEvent("fsChanged", (e) => {
const worktree = [...e.created, ...e.modified, ...e.removed].some((p) => !/(^|[/\\])\.git([/\\]|$)/.test(p));
if (worktree) scheduleRefresh(250);
});
const offSettings = onWorkflowSettingsChange(
(s) => s.vcs.showDirtyDirectories,
(show) => {