diff --git a/Source/FileManager/BackgroundFileSystem.cs b/Source/FileManager/BackgroundFileSystem.cs
index 4a319084..b1518d93 100644
--- a/Source/FileManager/BackgroundFileSystem.cs
+++ b/Source/FileManager/BackgroundFileSystem.cs
@@ -107,7 +107,17 @@ private void Stop()
catch (ObjectDisposedException) { }
//Wait for background scanner to terminate before reinitializing.
- backgroundScanner?.Wait();
+ try
+ {
+ backgroundScanner?.Wait();
+ }
+ // A scanner that died still has to be waited on, but its failure is not this caller's to raise: Stop() is
+ // reached from Refresh() and from Dispose(), neither of which has anything to do with whatever went wrong,
+ // and both of which are about to replace the scanner anyway.
+ catch (AggregateException ex)
+ {
+ Serilog.Log.Logger.Error(ex, "The file cache's background scanner had already stopped on an error.");
+ }
//Dispose of directoryChangesEvents after backgroundScanner exists. Clear the field first so a late event
//has nothing to add to. CompleteAdding has to have happened while it was still set, or the scanner would
@@ -146,8 +156,19 @@ private void BackgroundScanner()
{
while (directoryChangesEvents?.TryTake(out var change, -1) is true)
{
- lock (fsCacheLocker)
- UpdateLocalCache(change);
+ try
+ {
+ lock (fsCacheLocker)
+ UpdateLocalCache(change);
+ }
+ // One path this cannot read must not end the scan. An exception here used to be terminal twice over:
+ // it stopped the cache tracking anything further, and it was stored on the task, so the next Stop()
+ // rethrew it as an AggregateException at whoever had called Refresh() - a caller with nothing to do
+ // with the file that went missing.
+ catch (Exception ex)
+ {
+ Serilog.Log.Logger.Debug(ex, "Could not apply a file system change to the file cache: {@DebugText}", new { change.ChangeType, change.FullPath });
+ }
}
}
@@ -181,14 +202,36 @@ private void AddPath(LongPath path)
{
path = path.LongPathName;
//Temporary files created when updating the db will disappear before their attributes can be read.
- if (Path.GetFileName(path).Contains("LibationContext.db") || !File.Exists(path) && !Directory.Exists(path))
+ if (Path.GetFileName(path).Contains("LibationContext.db"))
+ return;
+
+ // Whether it exists and what it is were two questions, and the answer to the first could stop being true
+ // before the second was asked: a download's temp file, or a folder the user has just moved or deleted, is
+ // gone by the time its attributes are read. One question instead, and its failure means the same thing
+ // the existence check meant - there is nothing here to add.
+ if (TryGetAttributes(path) is not FileAttributes attributes)
return;
- if (File.GetAttributes(path).HasFlag(FileAttributes.Directory))
+
+ if (attributes.HasFlag(FileAttributes.Directory))
AddUniqueFiles(SafestEnumerateFiles(path));
else
AddUniqueFile(path);
}
+ /// What the path is, or null when it cannot be read and so has nothing to add.
+ internal static FileAttributes? TryGetAttributes(LongPath path)
+ {
+ try
+ {
+ return File.GetAttributes(path);
+ }
+ catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or ArgumentException or NotSupportedException)
+ {
+ Serilog.Log.Logger.Debug(ex, "Nothing to add to the file cache for {@DebugText}", new { path = (string)path });
+ return null;
+ }
+ }
+
private IEnumerable SafestEnumerateFiles(string path)
{
try
diff --git a/Source/FileManager/_InternalsVisible.cs b/Source/FileManager/_InternalsVisible.cs
new file mode 100644
index 00000000..47db7673
--- /dev/null
+++ b/Source/FileManager/_InternalsVisible.cs
@@ -0,0 +1 @@
+[assembly: System.Runtime.CompilerServices.InternalsVisibleTo(nameof(FileManager) + ".Tests")]
diff --git a/Source/_Tests/FileManager.Tests/BackgroundFileSystemTests.cs b/Source/_Tests/FileManager.Tests/BackgroundFileSystemTests.cs
index 4565db85..178b2cf1 100644
--- a/Source/_Tests/FileManager.Tests/BackgroundFileSystemTests.cs
+++ b/Source/_Tests/FileManager.Tests/BackgroundFileSystemTests.cs
@@ -103,3 +103,132 @@ private static bool WaitFor(Func condition, int timeoutMs = 5000)
return false;
}
}
+
+///
+/// A second failure mode in the same class, from a CI run where all three Windows legs failed and the other six
+/// passed: every test in FileLiberator.Tests' PDF path suite failed in TestInitialize with an AggregateException
+/// wrapping FileNotFoundException: Could not find file ..., naming a path none of those tests had
+/// anything to do with. The watcher had raised Created for a folder an earlier test's cleanup then deleted; the
+/// scanner asked whether it existed, was told yes, asked what it was, and got an exception. That killed the
+/// scanner and was stored on its task, and the next Stop() - reached from AudibleFileStorage.Audio.Refresh() by
+/// way of Dispose() - rethrew it at that caller.
+///
+/// The trigger is a disagreement between Exists and GetAttributes over a long \\?\ path, which cannot be
+/// staged on the platform these tests usually run on. So the guard itself is asserted directly, and the rest
+/// covers what the death cost: a cache that stops tracking, and an exception handed to an unrelated caller.
+///
+///
+[TestClass]
+[DoNotParallelize]
+public class PathsThatVanishBeforeTheyAreRead
+{
+ private string tempDir = string.Empty;
+
+ [TestInitialize]
+ public void Initialize()
+ {
+ tempDir = Path.Combine(Path.GetTempPath(), $"libation-bfs-vanishing-{Guid.NewGuid():N}");
+ Directory.CreateDirectory(tempDir);
+ }
+
+ [TestCleanup]
+ public void Cleanup()
+ {
+ try
+ {
+ Directory.Delete(tempDir, recursive: true);
+ }
+ catch (IOException)
+ {
+ // A leftover temp directory is not worth failing a test over.
+ }
+ }
+
+ /// Creates a batch of files and deletes the tree while their events are still queued.
+ private void ChurnVanishingFiles()
+ {
+ for (var batch = 0; batch < 5; batch++)
+ {
+ var directory = Path.Combine(tempDir, $"vanishing-{batch}");
+ Directory.CreateDirectory(directory);
+
+ for (var i = 0; i < 40; i++)
+ File.WriteAllText(Path.Combine(directory, $"book-{i}.m4b"), "audio");
+
+ // No wait: the scanner should reach these paths after they have gone.
+ Directory.Delete(directory, recursive: true);
+ }
+ }
+
+ [TestMethod]
+ public void a_path_that_is_not_there_reads_as_nothing_rather_than_throwing()
+ {
+ // The fix, on its own terms. Asking what a path is used to be allowed to throw, on the strength of having
+ // asked a moment earlier whether it was there.
+ Assert.IsNull(BackgroundFileSystem.TryGetAttributes(Path.Combine(tempDir, "never-existed.m4b")));
+ Assert.IsNull(BackgroundFileSystem.TryGetAttributes(Path.Combine(tempDir, "no", "such", "folder", "book.m4b")));
+ }
+
+ [TestMethod]
+ public void a_path_that_is_there_still_reads_as_what_it_is()
+ {
+ var file = Path.Combine(tempDir, "real.m4b");
+ File.WriteAllText(file, "audio");
+
+ Assert.IsFalse(BackgroundFileSystem.TryGetAttributes(file)!.Value.HasFlag(FileAttributes.Directory));
+ Assert.IsTrue(BackgroundFileSystem.TryGetAttributes(tempDir)!.Value.HasFlag(FileAttributes.Directory));
+ }
+
+ [TestMethod]
+ public void the_scanner_keeps_going_after_a_path_it_cannot_read()
+ {
+ using var sut = new BackgroundFileSystem(tempDir, "*.*", SearchOption.AllDirectories);
+
+ ChurnVanishingFiles();
+
+ // The scanner is still doing its job, which is what dying used to cost: the first unreadable path ended
+ // the loop, and every later change to the Books directory went unnoticed for the rest of the session.
+ File.WriteAllText(Path.Combine(tempDir, "survivor.m4b"), "audio");
+
+ var found = WaitFor(() => sut.FindFile(new Regex(@"survivor\.m4b$")) is not null);
+ Assert.IsTrue(found, "the scanner stopped tracking changes after a path it could not read");
+ }
+
+ [TestMethod]
+ public void disposing_afterwards_does_not_hand_the_failure_to_the_caller()
+ {
+ // This is the CI stack: Refresh() found the Books directory changed, disposed the old file system, and
+ // Stop() waited on a scanner that had already faulted.
+ var sut = new BackgroundFileSystem(tempDir, "*.*", SearchOption.AllDirectories);
+
+ ChurnVanishingFiles();
+
+ sut.Dispose();
+ }
+
+ [TestMethod]
+ public void a_file_that_outlives_its_event_is_still_tracked()
+ {
+ // The guard must not have been widened into ignoring everything: what is really there still lands.
+ using var sut = new BackgroundFileSystem(tempDir, "*.*", SearchOption.AllDirectories);
+
+ var directory = Path.Combine(tempDir, "kept");
+ Directory.CreateDirectory(directory);
+ File.WriteAllText(Path.Combine(directory, "kept.m4b"), "audio");
+
+ var found = WaitFor(() => sut.FindFile(new Regex(@"kept\.m4b$")) is not null);
+ Assert.IsTrue(found, "a file that was never deleted did not reach the cache");
+ }
+
+ private static bool WaitFor(Func condition, int timeoutMs = 5000)
+ {
+ for (var waited = 0; waited < timeoutMs; waited += 50)
+ {
+ if (condition())
+ return true;
+ Thread.Sleep(50);
+ }
+
+ return false;
+ }
+}