Skip to content

Commit acf2793

Browse files
Your Nameclaude
andcommitted
fix(test): prevent watcher_integration leaking threads/tempdirs on panic
Root-caused the 'all-languages' CI job's flaky/multi-hour hangs: each of the 3 tests in watcher_integration.rs spawns a background watcher thread, then does `ready_rx.recv_timeout(10s).expect(...)` before its later `ct.cancel(); handle.join(); remove_dir_all(...)` cleanup. Under the heavy parallel CPU/IO load the all-languages job creates (12 opt-in language grammars + scip-overlay + lsp-overlay compiled/tested together), the watcher's arm+initial-reconciliation step can miss that 10s window; the resulting panic unwinds past the cleanup lines entirely, leaking a live watcher thread and its temp project dir for the rest of the process's life. Reproduced locally under this same load (watcher_hot_reloads_ coverage_file_change failed with exactly this timeout). Adds a WatchGuard (Drop-based RAII) so cancel/join/cleanup always run, panic or not, and widens the ready-wait from 10s to 30s to match the other bounded waits already used in this file. This does not change crates/calm-server/src/watch_supervisor.rs's actual arm+reconcile behavior -- that's a separate, bigger design question (signal armed before vs. after reconciliation) intentionally left for a follow-up. Verified: cargo test -p calm-server --test watcher_integration (3/3 pass), cargo clippy -p calm-server --all-targets -- -D warnings clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent e42a5d0 commit acf2793

1 file changed

Lines changed: 50 additions & 18 deletions

File tree

crates/calm-server/tests/watcher_integration.rs

Lines changed: 50 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,32 @@ use std::time::{Duration, Instant};
99

1010
use tokio_util::sync::CancellationToken;
1111

12+
/// Ensures the watcher thread is always cancelled/joined and the temp
13+
/// project dir removed, even if an assertion earlier in the test body
14+
/// panics. A bare `ct.cancel(); handle.join(); remove_dir_all(...)`
15+
/// sequence placed at the end of a test never runs on panic (e.g. the
16+
/// `ready_rx.recv_timeout(...).expect(...)` below, which is exactly what
17+
/// timed out under the CPU contention of the `all-languages` CI job on
18+
/// 2026-08-01) -- that leaks a live background watcher thread plus its temp
19+
/// dir for the rest of the process's life, found while investigating a
20+
/// flaky/hanging CI run. `Drop` runs during unwind too, so wrapping the
21+
/// handle/dir in this guard makes cleanup unconditional.
22+
struct WatchGuard {
23+
ct: CancellationToken,
24+
handle: Option<std::thread::JoinHandle<()>>,
25+
dir: std::path::PathBuf,
26+
}
27+
28+
impl Drop for WatchGuard {
29+
fn drop(&mut self) {
30+
self.ct.cancel();
31+
if let Some(h) = self.handle.take() {
32+
let _ = h.join();
33+
}
34+
let _ = std::fs::remove_dir_all(&self.dir);
35+
}
36+
}
37+
1238
fn symbol_count(db: &Path) -> i64 {
1339
let conn = rusqlite::Connection::open(db).unwrap();
1440
conn.query_row("SELECT COUNT(*) FROM symbols", [], |r| r.get(0))
@@ -76,12 +102,18 @@ fn watcher_reindexes_add_and_delete() {
76102
})
77103
};
78104

105+
let _guard = WatchGuard {
106+
ct: ct.clone(),
107+
handle: Some(handle),
108+
dir: dir.clone(),
109+
};
110+
79111
// Wait for the watcher to actually arm its OS-level watch before mutating
80112
// the tree — a fixed sleep here raced real thread-scheduling delay under
81113
// load (see `run_watch_loop`'s `ready` doc comment).
82114
ready_rx
83-
.recv_timeout(Duration::from_secs(10))
84-
.expect("watcher should signal ready within 10s");
115+
.recv_timeout(Duration::from_secs(30))
116+
.expect("watcher should signal ready within 30s");
85117

86118
// Add a file → watcher should incrementally index it.
87119
std::fs::write(dir.join("b.py"), "def b():\n pass\n").unwrap();
@@ -91,10 +123,6 @@ fn watcher_reindexes_add_and_delete() {
91123
std::fs::remove_file(dir.join("b.py")).unwrap();
92124
let removed = wait_for_symbols(&db_path, 1, Duration::from_secs(30));
93125

94-
ct.cancel();
95-
let _ = handle.join();
96-
let _ = std::fs::remove_dir_all(&dir);
97-
98126
assert!(added, "watcher should have indexed the added file");
99127
assert!(removed, "watcher should have dropped the deleted file");
100128
}
@@ -157,9 +185,15 @@ fn concurrent_edit_write_and_watcher_reindex_does_not_lock_or_go_stale() {
157185
)
158186
})
159187
};
188+
189+
let _guard = WatchGuard {
190+
ct: ct.clone(),
191+
handle: Some(handle),
192+
dir: dir.clone(),
193+
};
160194
ready_rx
161-
.recv_timeout(Duration::from_secs(10))
162-
.expect("watcher should signal ready within 10s");
195+
.recv_timeout(Duration::from_secs(30))
196+
.expect("watcher should signal ready within 30s");
163197

164198
// Simulate edit_lines_impl's own write+reindex sequence, firing right
165199
// after a file write the watcher is independently about to react to —
@@ -186,10 +220,6 @@ fn concurrent_edit_write_and_watcher_reindex_does_not_lock_or_go_stale() {
186220
// not leave the DB stale relative to disk.
187221
let converged = wait_for_symbols(&db_path, 2, Duration::from_secs(15));
188222

189-
ct.cancel();
190-
let _ = handle.join();
191-
let _ = std::fs::remove_dir_all(&dir);
192-
193223
assert!(
194224
converged,
195225
"DB must reflect the latest file content once both writers settle"
@@ -274,9 +304,15 @@ fn watcher_hot_reloads_coverage_file_change() {
274304
})
275305
};
276306

307+
let _guard = WatchGuard {
308+
ct: ct.clone(),
309+
handle: Some(handle),
310+
dir: dir.clone(),
311+
};
312+
277313
ready_rx
278-
.recv_timeout(Duration::from_secs(10))
279-
.expect("watcher should signal ready within 10s");
314+
.recv_timeout(Duration::from_secs(30))
315+
.expect("watcher should signal ready within 30s");
280316

281317
// Write a coverage report *after* the watcher is running — this is the
282318
// "regenerated mid-session" case the fix targets.
@@ -288,10 +324,6 @@ fn watcher_hot_reloads_coverage_file_change() {
288324

289325
let reloaded = wait_for_coverage_source(&coverage, "lcov", Duration::from_secs(30));
290326

291-
ct.cancel();
292-
let _ = handle.join();
293-
let _ = std::fs::remove_dir_all(&dir);
294-
295327
assert!(
296328
reloaded,
297329
"watcher should have hot-reloaded the new lcov.info without a restart"

0 commit comments

Comments
 (0)