Skip to content

Commit 419c493

Browse files
ionite34ionite34
authored
Merge pull request #1743 from NeuralFault/fix/model-index-visit-key-collision
Fix: [LinkSafeFileSystem] stop links from shadowing real folders in EnumerateFiles
2 parents fcfab8b + 8525cdf commit 419c493

2 files changed

Lines changed: 192 additions & 18 deletions

File tree

‎StabilityMatrix.Core/Helper/LinkSafeFileSystem.cs‎

Lines changed: 71 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -98,26 +98,79 @@ public static bool WouldLinkCycle(DirectoryPath sourceDir, DirectoryPath linkPat
9898

9999
/// <summary>
100100
/// Recursively enumerates files matching <paramref name="searchPattern"/> under
101-
/// <paramref name="rootDir"/>. Linked directories are followed once; a link back to a directory
102-
/// already visited is skipped, as are directories deeper than <paramref name="maxDepth"/>.
103-
/// Inaccessible directories are skipped rather than aborting the enumeration.
104-
/// Yielded paths are rooted at <paramref name="rootDir"/> as given, not at its resolved target.
101+
/// <paramref name="rootDir"/>. Symbolic links are followed, but every physical directory is
102+
/// visited at most once, so no file is yielded twice. Directories nested deeper than
103+
/// <paramref name="maxDepth"/> and directories that cannot be read are skipped without aborting
104+
/// the enumeration. Yielded paths are rooted at <paramref name="rootDir"/> as given, not at its
105+
/// resolved target.
105106
/// </summary>
106107
public static IEnumerable<string> EnumerateFiles(
107108
string rootDir,
108109
string searchPattern,
109110
int maxDepth = DefaultMaxDepth
110111
)
111112
{
112-
var visited = new HashSet<string>(PathComparer);
113-
var pending = new Stack<(string Path, string RealPath, int Depth)>();
113+
// A real directory is keyed by its literal path, compared ordinally, so two folders whose
114+
// names differ only in case are both scanned; it is also matched against link targets, so a
115+
// real folder reached through a link is not rescanned. A link is keyed by its resolved
116+
// target, compared with the platform's case sensitivity (PathComparer), because a target is
117+
// stored however the link was created.
118+
var visitedRealDirsExact = new Dictionary<string, string>(StringComparer.Ordinal);
119+
var visitedRealDirsForLinkTargets = new Dictionary<string, string>(PathComparer);
120+
var visitedLinkTargets = new Dictionary<string, string>(PathComparer);
121+
122+
// Real directories are drained to completion before any link is considered, so a link can
123+
// never take the identity of a real folder and shadow it out of the scan.
124+
var realDirs = new Stack<(string Path, string RealPath, int Depth)>();
125+
var linkedDirs = new Stack<(string Path, string RealPath, int Depth)>();
114126

115127
var rootReal = GetRealPath(rootDir);
116-
visited.Add(rootReal);
117-
pending.Push((rootDir, rootReal, 0));
128+
realDirs.Push((rootDir, rootReal, 0));
118129

119-
while (pending.TryPop(out var dir))
130+
while (realDirs.Count > 0 || linkedDirs.Count > 0)
120131
{
132+
// Which stack the entry came from is how the walk knows whether it is a link.
133+
var fromRealDirs = realDirs.Count > 0;
134+
var dir = fromRealDirs ? realDirs.Pop() : linkedDirs.Pop();
135+
136+
// Claimed on pop, not on push, so the walk order decides which spelling owns the
137+
// identity instead of the reversed push order.
138+
if (!fromRealDirs)
139+
{
140+
if (
141+
visitedLinkTargets.TryGetValue(dir.RealPath, out var linkClaimer)
142+
|| visitedRealDirsForLinkTargets.TryGetValue(dir.RealPath, out linkClaimer)
143+
)
144+
{
145+
Logger.Info(
146+
"Skipping {Path}: the same directory was already scanned as {ClaimedBy}",
147+
dir.Path,
148+
linkClaimer
149+
);
150+
continue;
151+
}
152+
153+
visitedLinkTargets[dir.RealPath] = dir.Path;
154+
}
155+
else
156+
{
157+
if (
158+
visitedRealDirsExact.TryGetValue(dir.RealPath, out var realClaimer)
159+
|| visitedLinkTargets.TryGetValue(dir.RealPath, out realClaimer)
160+
)
161+
{
162+
Logger.Warn(
163+
"Skipping {Path}: the same directory was already scanned as {ClaimedBy}",
164+
dir.Path,
165+
realClaimer
166+
);
167+
continue;
168+
}
169+
170+
visitedRealDirsExact[dir.RealPath] = dir.Path;
171+
visitedRealDirsForLinkTargets[dir.RealPath] = dir.Path;
172+
}
173+
121174
List<string> files;
122175
List<DirectoryInfo> subDirs;
123176
try
@@ -150,21 +203,21 @@ public static IEnumerable<string> EnumerateFiles(
150203
continue;
151204
}
152205

153-
// Pushed in reverse so the stack pops them in enumeration order
206+
// Pushed in reverse so each stack pops its entries in enumeration order
154207
for (var i = subDirs.Count - 1; i >= 0; i--)
155208
{
156209
var subDir = subDirs[i];
157-
var subReal = subDir.Attributes.HasFlag(FileAttributes.ReparsePoint)
158-
? GetRealPath(subDir.FullName)
159-
: Path.Join(dir.RealPath, subDir.Name);
210+
var isLinkDir = subDir.Attributes.HasFlag(FileAttributes.ReparsePoint);
211+
var subReal = isLinkDir ? GetRealPath(subDir.FullName) : Path.Join(dir.RealPath, subDir.Name);
160212

161-
if (!visited.Add(subReal))
213+
if (isLinkDir)
162214
{
163-
Logger.Debug("Skipping {Path}: already visited as {RealPath}", subDir.FullName, subReal);
164-
continue;
215+
linkedDirs.Push((subDir.FullName, subReal, dir.Depth + 1));
216+
}
217+
else
218+
{
219+
realDirs.Push((subDir.FullName, subReal, dir.Depth + 1));
165220
}
166-
167-
pending.Push((subDir.FullName, subReal, dir.Depth + 1));
168221
}
169222
}
170223
}

‎StabilityMatrix.Tests/Helper/LinkSafeFileSystemTests.cs‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,127 @@ public void EnumerateFiles_TwoLinksToSameDirectory_VisitsItOnce()
100100
Assert.AreEqual(1, files.Count);
101101
}
102102

103+
[DataTestMethod]
104+
[DataRow(true)]
105+
[DataRow(false)]
106+
public void EnumerateFiles_RealDirAlreadyClaimedAsLinkTarget_IsVisitedOnce(bool xyLinkIsDeeper)
107+
{
108+
// ext/X/Y/y.json is reachable two ways: through a link to ext/X/Y, and as a real subfolder
109+
// of a link to ext/X. Sibling enumeration order is file-system dependent, so nesting the
110+
// links at different depths pins the walk order instead of relying on names: the deeper
111+
// link is always popped first (draining real dirs first pushes it last, and the link stack
112+
// pops last-in first-out).
113+
var root = CreateDir("root");
114+
var sub = CreateDir("root", "sub");
115+
CreateFile("ext", "X", "Y", "y.json");
116+
117+
var x = Path.Combine(tempDir, "ext", "X");
118+
var xy = Path.Combine(x, "Y");
119+
120+
// Deep link -> ext/X/Y, shallow link -> ext/X. When the deep link is the one targeting
121+
// ext/X/Y, it is walked first and the shallower ext/X link then reaches that same real
122+
// folder again through its "Y" child.
123+
var xyLink = Path.Combine(xyLinkIsDeeper ? sub : root, "inner");
124+
var xLink = Path.Combine(xyLinkIsDeeper ? root : sub, "outer");
125+
TempFiles.CreateDirectoryLink(xyLink, xy);
126+
TempFiles.CreateDirectoryLink(xLink, x);
127+
128+
var files = LinkSafeFileSystem.EnumerateFiles(root, "*.json").ToList();
129+
130+
Assert.AreEqual(1, files.Count, $"Expected one file, got: {string.Join(", ", files)}");
131+
}
132+
133+
[DataTestMethod]
134+
[DataRow("a_inner", "b_outer")]
135+
[DataRow("b_inner", "a_outer")]
136+
public void EnumerateFiles_NestedLinkTarget_SiblingLinkOrder_IsVisitedOnce(
137+
string innerName,
138+
string outerName
139+
)
140+
{
141+
if (!Compat.IsWindows)
142+
{
143+
Assert.Inconclusive(
144+
"Needs NTFS, which enumerates sibling directories in stored name order; "
145+
+ "EnumerateFiles_RealDirAlreadyClaimedAsLinkTarget_IsVisitedOnce covers the same "
146+
+ "bug portably by varying depth instead."
147+
);
148+
return;
149+
}
150+
151+
// The maintainer's original repro: innerName -> ext/X/Y, outerName -> ext/X, so the inner
152+
// link's target is also reached as a real subfolder of the outer link. NTFS yields siblings
153+
// in name order, so the two rows walk the links in opposite orders.
154+
var root = CreateDir("root");
155+
CreateFile("ext", "X", "Y", "y.json");
156+
157+
var x = Path.Combine(tempDir, "ext", "X");
158+
var xy = Path.Combine(x, "Y");
159+
160+
TempFiles.CreateDirectoryLink(Path.Combine(root, innerName), xy);
161+
TempFiles.CreateDirectoryLink(Path.Combine(root, outerName), x);
162+
163+
var files = LinkSafeFileSystem.EnumerateFiles(root, "*.json").ToList();
164+
165+
Assert.AreEqual(1, files.Count, $"Expected one file, got: {string.Join(", ", files)}");
166+
}
167+
168+
[DataTestMethod]
169+
[DataRow("diffusion_models")]
170+
[DataRow("sub", "alias")]
171+
public void EnumerateFiles_RealFolderShadowedByLink_KeepsRealFolderPaths(params string[] linkSegments)
172+
{
173+
var root = CreateDir("root");
174+
CreateFile("root", "DiffusionModels", "a.json");
175+
CreateFile("root", "DiffusionModels", "b.json");
176+
177+
var linkPath = Path.Combine([root, .. linkSegments]);
178+
Directory.CreateDirectory(Path.GetDirectoryName(linkPath)!);
179+
TempFiles.CreateDirectoryLink(linkPath, Path.Combine(root, "DiffusionModels"));
180+
181+
var files = LinkSafeFileSystem.EnumerateFiles(root, "*.json").ToList();
182+
183+
CollectionAssert.AreEquivalent(
184+
new[]
185+
{
186+
Path.Combine(root, "DiffusionModels", "a.json"),
187+
Path.Combine(root, "DiffusionModels", "b.json"),
188+
},
189+
files
190+
);
191+
}
192+
193+
[TestMethod]
194+
public void EnumerateFiles_JunctionTargetCaseMismatch_KeepsRealFolderPaths()
195+
{
196+
if (!Compat.IsWindows)
197+
{
198+
Assert.Inconclusive("Junctions with a differently-cased stored target are Windows-only.");
199+
return;
200+
}
201+
202+
var root = CreateDir("root");
203+
CreateFile("root", "DiffusionModels", "a.json");
204+
CreateFile("root", "DiffusionModels", "b.json");
205+
206+
// Store the junction target with different casing than the real folder on disk.
207+
TempFiles.CreateDirectoryLink(
208+
Path.Combine(root, "diffusion_models"),
209+
Path.Combine(root.ToUpperInvariant(), "DIFFUSIONMODELS")
210+
);
211+
212+
var files = LinkSafeFileSystem.EnumerateFiles(root, "*.json").ToList();
213+
214+
CollectionAssert.AreEquivalent(
215+
new[]
216+
{
217+
Path.Combine(root, "DiffusionModels", "a.json"),
218+
Path.Combine(root, "DiffusionModels", "b.json"),
219+
},
220+
files
221+
);
222+
}
223+
103224
[TestMethod]
104225
public void EnumerateFiles_DeeperThanMaxDepth_IsSkipped()
105226
{

0 commit comments

Comments
 (0)