Skip to content

Commit 4ade2cf

Browse files
authored
Turbopack: Watch parent directories before watching children in non_recursive_helpers::start_watching_dir_and_parents (#82454)
Fixes a potential race condition #82130 (comment): > I think there is a race condition here, when the child dir is created while we are walking the parent directories. > > 1. we try to watch /path/to/dir but it doesn't exist. > 2. /path/to/dir is created > 3. we move to the parent directory (`path/to`) and start watching it. > 4. But we already missed the event that path/to/dir was created. > > I think we need to run the loop reverse direction, watching parent directories first before watching the child directories. This way we already have the parent directory watcher setup before "reading" the child directory (reading in the sense of starting to watch it). Tested by putting a `println!()` in `start_watching_dir` and observing that the parents were watched first when running ``` rm -rf /tmp/fuzz && cargo run -p turbo-tasks-fuzz -- fs-watcher --fs-root /tmp/fuzz ```
1 parent 4984ac4 commit 4ade2cf

File tree

1 file changed

+32
-16
lines changed

1 file changed

+32
-16
lines changed

turbopack/crates/turbo-tasks-fs/src/watcher.rs

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -287,23 +287,39 @@ mod non_recursive_helpers {
287287
dir_path: &Path,
288288
root_path: &Path,
289289
) -> Result<()> {
290-
let mut cur_path = dir_path;
291-
loop {
292-
start_watching_dir(&mut state.notify_watcher, cur_path, root_path)?;
293-
294-
let Some(parent_path) = cur_path.parent() else {
295-
// this should never happen as we break before we reach the root path
296-
anyhow::bail!(
297-
"failed to compute parent path of {cur_path:?} while watching {dir_path:?} in \
298-
root {root_path:?}"
299-
);
300-
};
301-
302-
if parent_path == root_path || !state.watched.insert(parent_path.to_path_buf()) {
303-
break;
304-
}
290+
let mut found_watched_ancestor = false;
291+
292+
// NOTE: `Path::ancestors` yields ancestors from longest to shortest path.
293+
let dir_and_ancestor_paths: Vec<_> = [dir_path]
294+
.into_iter()
295+
.chain(
296+
dir_path
297+
.ancestors()
298+
// skip: `ancestors` includes `dir_path` itself, as well as the ancestors, but
299+
// we only want to apply the `take_while` check to parents
300+
.skip(1)
301+
.take_while(|p| {
302+
found_watched_ancestor = *p == root_path || state.watched.contains(*p);
303+
!found_watched_ancestor
304+
}),
305+
)
306+
.collect();
307+
308+
if !found_watched_ancestor {
309+
// this should never happen, as we should eventually hit the `root_path`
310+
anyhow::bail!(
311+
"failed to find the fs root of {root_path:?} while watching {dir_path:?}"
312+
);
313+
}
305314

306-
cur_path = parent_path;
315+
// Reverse the iterator: We want to start closest to the root and work towards `dir_path`
316+
// (opposite of `Path::ancestors`), to avoid a potential race condition if directories are
317+
// removed and re-added before we've watched their parent.
318+
for path in dir_and_ancestor_paths.into_iter().rev() {
319+
// this will silently ignore if the path is not found, expecting that we've watched the
320+
// parent directory
321+
start_watching_dir(&mut state.notify_watcher, path, root_path)?;
322+
state.watched.insert(path.to_owned());
307323
}
308324

309325
Ok(())

0 commit comments

Comments
 (0)