SaferEnumerateFiles returned a lazy sequence, so an I/O error was raised where
the sequence was walked rather than where it was created - past the try/catch
callers had wrapped around it. IgnoreInaccessible did not help either: it only
forgives permissions, not a volume that has stopped answering.
Walk the enumerator defensively instead, keeping what was read and reporting
the reason, and let a caller ask whether a directory can be read at all.
See issue #1984.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
All three Windows CI legs failed on master while the other six passed, and not on
an assertion: every test in FileLiberator.Tests' PDF path suite failed in
TestInitialize with an AggregateException wrapping FileNotFoundException, 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. AddPath asked whether the path existed, was told yes, asked what it was,
and got an exception - Exists and GetAttributes can disagree over a long \\?\
path, and the answer to the first can stop being true before the second is asked
anyway. That exception ended the background scanner, so nothing further reached
the cache, and it was stored on the task, so the next Stop() rethrew it as an
AggregateException at whoever had called Refresh(). In the app that caller is the
Books directory refresh after every download.
Three changes, smallest first: the attribute read is guarded and returns 'nothing
to add' where the existence check used to say it, which also removes the race
rather than narrowing it; the scanner survives an event it cannot apply; and
Stop() waits on a scanner that has already failed without handing the failure to
a caller that is about to replace it.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
A Windows CI leg failed with every test passing. FileLiberator.Tests
exited 0xE0434352:
Unhandled exception. System.InvalidOperationException: The collection has
been marked as complete with regards to additions.
at BlockingCollection`1.Add(T item)
at FileManager.BackgroundFileSystem.FileSystemWatcher_Changed(...)
at FileSystemWatcher.ReadDirectoryChangesCallback(...)
Stop() disposes the watcher and then completes the collection, on the
assumption that disposing stops events. It does not stop the ones the OS
has already buffered, and on Windows those arrive on a native completion
callback, where an exception is not a failed call - it is a dead process.
So a Libation run that reinitialises or shuts down its file cache while
the Books directory is busy can take the app with it, which is the same
race the tests hit.
Adding to a completed collection is now caught and the event dropped.
That is the right answer rather than a swallow: whoever called Stop() is
either reinitialising, which rebuilds the cache from disk, or disposing.
Stop() also detaches its handlers before disposing and clears the fields,
which narrows the window and makes a second Stop() harmless - the
collection field is cleared only after CompleteAdding, since the
background scanner is waiting on that.
First tests for the class, since it had none: dispose under a flood of
events, dispose twice, and find a file created before and after
construction. They cannot prove this fix - the crash does not reproduce on
Linux even with the original code, because inotify does not deliver
post-dispose events the way Windows does. Windows CI is the only place
that can, so the guard is aimed there.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
