Skip to content

Commit a97dc2c

Browse files
authored
Fix symlink loop performance. (#2232)
1 parent 274e96c commit a97dc2c

File tree

9 files changed

+443
-85
lines changed

9 files changed

+443
-85
lines changed

pkgs/watcher/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
- Polling watchers now check file sizes as well as "last modified" times, so
44
they are less likely to miss changes on platforms with low resolution
55
timestamps.
6+
- Bug fix: while listing directories skip symlinks that lead to a directory
7+
that has already been listed. This prevents a severe performance regression on
8+
MacOS and Linux when there are more than a few symlink loops.
69
- Bug fix: with `FileWatcher` on MacOS, a modify event was sometimes reported if
710
the file was created immediately before the watcher was created. Now, if the
811
file exists when the watcher is created then this modify event is not sent.
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:async';
6+
import 'dart:io';
7+
8+
import 'package:path/path.dart' as p;
9+
10+
import '../utils.dart';
11+
12+
extension DirectoryRobustRecursiveListing on Directory {
13+
/// Lists the given directory recursively ignoring not-found or access errors.
14+
///
15+
/// These can arise from concurrent file-system modification.
16+
///
17+
/// See [listRecursively] for how symlinks are handled.
18+
Stream<FileSystemEntity> listRecursivelyIgnoringErrors() {
19+
return listRecursively()
20+
.ignoring<PathNotFoundException>()
21+
.ignoring<PathAccessException>();
22+
}
23+
24+
/// Lists the directory recursively.
25+
///
26+
/// This is like `Directory.list(recursive: true)`, but handles symlinks like
27+
/// `find -L` to avoid a performance issue with symbolic link cycles.
28+
///
29+
/// See: https://github.com/dart-lang/sdk/issues/61407.
30+
///
31+
/// A link to a directory is only followed if the link target is not currently
32+
/// being traversed. For this check, directories are compared using their
33+
/// symlink-resolved paths.
34+
///
35+
/// Skipped links to directories are not mentioned in the directory listing.
36+
Stream<FileSystemEntity> listRecursively() =>
37+
_DirectoryTraversal(this).listRecursively();
38+
}
39+
40+
/// A recursive directory listing algorithm that follows symlinks carefully.
41+
class _DirectoryTraversal {
42+
final Directory root;
43+
final StreamController<FileSystemEntity> _result = StreamController();
44+
45+
/// The directories currently being traversed.
46+
///
47+
/// These are canonical paths with symlinks resolved, for correct comparison.
48+
final Set<String> _traversing = {};
49+
50+
_DirectoryTraversal(this.root);
51+
52+
Stream<FileSystemEntity> listRecursively() {
53+
unawaited(_listAndRecurse());
54+
return _result.stream;
55+
}
56+
57+
/// Lists [root] then closes [_result].
58+
Future<void> _listAndRecurse() async {
59+
try {
60+
final resolvedRoot = _ResolvedDirectory.resolve(root);
61+
_traversing.add(resolvedRoot.canonicalPath);
62+
await _listAndRecurseOrThrow(resolvedRoot);
63+
} catch (e, s) {
64+
_result.addError(e, s);
65+
} finally {
66+
await _result.close();
67+
}
68+
}
69+
70+
/// Lists [directory] and its subdirectories.
71+
///
72+
/// A subdirectory is only listed if its canonical path is not already in
73+
/// [_traversing].
74+
Future<void> _listAndRecurseOrThrow(_ResolvedDirectory directory) async {
75+
final subdirectories = <_ResolvedDirectory>[];
76+
77+
await for (var entity
78+
in directory.directory.list(recursive: false, followLinks: false)) {
79+
// Handle links.
80+
if (entity is Link) {
81+
// Look up their target and target type.
82+
final target = entity.targetSync();
83+
final targetType = FileSystemEntity.typeSync(target);
84+
85+
if (targetType == FileSystemEntityType.directory) {
86+
// Add links to directories with their target to [subdirectories].
87+
subdirectories.add(_ResolvedDirectory(
88+
directory: Directory(entity.path), canonicalPath: target));
89+
} else if (targetType == FileSystemEntityType.file) {
90+
// Output files.
91+
_result.add(File(entity.path));
92+
} else {
93+
// Anything else. Broken links get output with type `Link`.
94+
_result.add(entity);
95+
}
96+
continue;
97+
}
98+
99+
// Handle directories.
100+
if (entity is Directory) {
101+
// If [directory] is canonical, a subdirectory within it is already
102+
// canonical.
103+
//
104+
// If [directory] is _not_ canonical, construct the canonical path of
105+
// the subdirectory by joining its basename to
106+
// [directory.resolvedDirectory].
107+
final resolvedDirectory = directory.isCanonical
108+
? entity.path
109+
: p.join(directory.canonicalPath, p.basename(entity.path));
110+
subdirectories.add(_ResolvedDirectory(
111+
directory: entity, canonicalPath: resolvedDirectory));
112+
continue;
113+
}
114+
115+
// Files and anything else.
116+
_result.add(entity);
117+
}
118+
119+
// Recurse into subdirectories that are not already being traversed.
120+
for (final directory in subdirectories) {
121+
if (_traversing.add(directory.canonicalPath)) {
122+
_result.add(directory.directory);
123+
await _listAndRecurseOrThrow(directory);
124+
_traversing.remove(directory.canonicalPath);
125+
}
126+
}
127+
}
128+
}
129+
130+
/// A directory plus its canonical path.
131+
class _ResolvedDirectory {
132+
final Directory directory;
133+
final String canonicalPath;
134+
135+
_ResolvedDirectory({required this.directory, required this.canonicalPath});
136+
137+
static _ResolvedDirectory resolve(Directory directory) {
138+
try {
139+
return _ResolvedDirectory(
140+
directory: directory,
141+
canonicalPath: directory.resolveSymbolicLinksSync());
142+
} on FileSystemException catch (e, s) {
143+
// The first operation on a directory is to resolve symbolic links, which
144+
// fails with a general FileSystemException if the file is not found.
145+
// Convert that into a PathNotFoundException as that makes more sense
146+
// to the caller, who didn't ask for anything to do with symbolic links.
147+
if (e.message.contains('Cannot resolve symbolic links') &&
148+
e.osError?.errorCode == 2) {
149+
throw Error.throwWithStackTrace(
150+
PathNotFoundException(directory.path, e.osError!), s);
151+
}
152+
rethrow;
153+
}
154+
}
155+
156+
bool get isCanonical => canonicalPath == directory.path;
157+
}

pkgs/watcher/lib/src/directory_watcher/linux.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import '../path_set.dart';
1313
import '../resubscribable.dart';
1414
import '../utils.dart';
1515
import '../watch_event.dart';
16+
import 'directory_list.dart';
1617

1718
/// Uses the inotify subsystem to watch for filesystem events.
1819
///
@@ -239,7 +240,8 @@ class _LinuxDirectoryWatcher
239240

240241
/// Emits [ChangeType.ADD] events for the recursive contents of [path].
241242
void _addSubdir(String path) {
242-
_listen(Directory(path).list(recursive: true), (FileSystemEntity entity) {
243+
_listen(Directory(path).listRecursivelyIgnoringErrors(),
244+
(FileSystemEntity entity) {
243245
if (entity is Directory) {
244246
_watchSubdir(entity.path);
245247
} else {

pkgs/watcher/lib/src/directory_watcher/mac_os.dart

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import '../path_set.dart';
1313
import '../resubscribable.dart';
1414
import '../utils.dart';
1515
import '../watch_event.dart';
16+
import 'directory_list.dart';
1617

1718
/// Uses the FSEvents subsystem to watch for filesystem events.
1819
///
@@ -148,9 +149,7 @@ class _MacOSDirectoryWatcher
148149
case EventType.createDirectory:
149150
if (_files.containsDir(path)) continue;
150151

151-
var stream = Directory(path)
152-
.list(recursive: true)
153-
.ignoring<PathNotFoundException>();
152+
var stream = Directory(path).listRecursivelyIgnoringErrors();
154153
var subscription = stream.listen((entity) {
155154
if (entity is Directory) return;
156155
if (_files.contains(path)) return;

pkgs/watcher/lib/src/directory_watcher/polling.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import '../polling.dart';
1111
import '../resubscribable.dart';
1212
import '../utils.dart';
1313
import '../watch_event.dart';
14+
import 'directory_list.dart';
1415

1516
/// Periodically polls a directory for changes.
1617
///

pkgs/watcher/lib/src/directory_watcher/windows.dart

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import '../path_set.dart';
1515
import '../resubscribable.dart';
1616
import '../utils.dart';
1717
import '../watch_event.dart';
18+
import 'directory_list.dart';
1819

1920
class WindowsDirectoryWatcher extends ResubscribableWatcher
2021
implements DirectoryWatcher {
@@ -216,9 +217,7 @@ class _WindowsDirectoryWatcher
216217
// that get removed during the `list` are already ignored by `list`
217218
// itself, so there are no other types of "path not found" that
218219
// might need different handling here.
219-
var stream = Directory(path)
220-
.list(recursive: true)
221-
.ignoring<PathNotFoundException>();
220+
var stream = Directory(path).listRecursivelyIgnoringErrors();
222221
var subscription = stream.listen((entity) {
223222
if (entity is Directory) return;
224223
if (_files.contains(entity.path)) return;

pkgs/watcher/lib/src/utils.dart

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,3 @@ extension IgnoringError<T> on Stream<T> {
7272
));
7373
}
7474
}
75-
76-
extension DirectoryRobustRecursiveListing on Directory {
77-
/// List the given directory recursively but ignore not-found or access
78-
/// errors.
79-
///
80-
/// Theses can arise from concurrent file-system modification.
81-
Stream<FileSystemEntity> listRecursivelyIgnoringErrors() {
82-
return list(recursive: true)
83-
.ignoring<PathNotFoundException>()
84-
.ignoring<PathAccessException>();
85-
}
86-
}

0 commit comments

Comments
 (0)