I have a class like this:
public class MyFolderRefresher {
public async Task OnRename(RenamedEventArgs e) {
// do something
}
}
public class MyDisposableListOfFileSystemWatchers : IDisposable {
private List<FileSystemWatcher> _mWatchers;
private MyFolderRefresher _mFolderRefresher;
public MyDisposableListOfFileSystemWatchers(List<string> pathsToWatch) {
_mFolderRefresher = new MyFolderRefresher();
_mWatchers = new List<FileSystemWatcher>();
foreach (var path in pathsToWatch) {
var fileSystemWatcher = new FileSystemWatcher(path)
{
NotifyFilter = NotifyFilters.DirectoryName,
IncludeSubdirectories = true,
InternalBufferSize = 64 * 1024
};
// Listen to events in some different class
fileSystemWatcher.Renamed += async (sender, e) => await _mFolderRefresher.OnRename(e);
_mWatchers.Add(fileSystemWatcher);
}
}
public void Dispose() {
Dispose(true);
}
private bool _mDisposed = false;
protected virtual void Dispose(bool disposing) {
if (_mDisposed) {
return;
}
if (disposing) {
foreach(var watcher in _mWatchers) {
watcher.Dispose();
}
_mWatchers.Clear();
}
_mDisposed = true;
}
}
The problem is that when calling Dispose()
of my MyDisposableListOfFileSystemWatchers class, I sometimes get System.InvalidOperationException
. Most of the time it's just a stack trace in the event viewer, but once I caught it in a debugger and was able to see that it was System.InvalidOperationException: Collection was modified; enumeration operation may not execute
(not sure if both are the same issue or not).
I'm wondering if I'm doing something wrong in my Dispose(). Specifically:
- Should I be calling
GC.SuppressFinalize(this)
in mypublic void Dispose()
? - Is it ok that I Dispose objects that I'm iterating over?
- Is it ok that I clear the collection, or should I leave that to the Garbage Collector?
- Since I'm Disposing the Publisher (FileSystemWatcher), am I correct in assuming I don't need to manually unsubscribe from the events?
I've read the documentation but wasn't able to find a definitive answer to those questions and I'm at a bit of a loss when it comes to the System.InvalidOperationException
as I don't see how it would be possible that both the constructor and Dispose could be running at the same time (those are the only places where _mWatchers` is modified).
_mWatchers
.for
loop instead? It may still throw an exception somewhere, but it could help you debug the problem betterMyFolderRefresher
or how you construct and dispose ofMyDisposableListOfFileSystemWatchers
so it will be hard for us to answer your question. Is there a chance you are callingMyDisposableListOfFileSystemWatchers.Dispose()
from multiple threads at once? If so that could definitely cause the problem you are seeing.GC.SuppressFinalize(this)
in mypublic void Dispose()
? -- You should implementDispose()
as shown in the MSFT docs here, which do include callingGC.SuppressFinalize(this);
. Your class only has managed resources to dispose which simplifies things. See Implementing IDisposable correctly.