Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 43bfad6

Browse files
committed
Fix ObjectDisposedException in FileSystemWatcher tests
In my code review of @sokket's new FileSystemWatcher tests (#2347), I'd encouraged him to clean up some of the AutoResetEvents that were being created, in the name of good hygiene. But this introduces a sporadic failure in the tests due to a race condition. The way that FileSystemWatcher is designed, stopping events on the instance doesn't actually prevent any in-progress operations from being processed, nor does stopping block until all in-process events have completed. As such, if after stopping processing we dispose of an event that's going to be set by one of the callbacks, it's possible such a set could occur from an in-flight operation after we've already disposed of the event, leading to an ObjectDisposedException that crashes the tests. I contemplated adding synchronization to FSW itself to avoid these kinds of race conditions, e.g. by ensuring that the call to set "Enabled = false;" won't return until all in-progress operations have completed, but that could cause deadlocks not currently possible. As such, I simply stopped explicitly disposing of the events created in the tests, leaving the cleanup to finalization (as is already done for most other events in the tests).
1 parent 1bcb7dc commit 43bfad6

File tree

2 files changed

+12
-9
lines changed

2 files changed

+12
-9
lines changed

src/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Deleted.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,10 @@ public static void FileSystemWatcher_Deleted_NestedDirectories()
8686
{
8787
using (var dir = Utility.CreateTestDirectory())
8888
using (var watcher = new FileSystemWatcher())
89-
using (AutoResetEvent createOccured = Utility.WatchForEvents(watcher, WatcherChangeTypes.Created))
90-
using (AutoResetEvent eventOccured = Utility.WatchForEvents(watcher, WatcherChangeTypes.Deleted))
9189
{
90+
AutoResetEvent createOccured = Utility.WatchForEvents(watcher, WatcherChangeTypes.Created); // not "using" to avoid race conditions with FSW callbacks
91+
AutoResetEvent eventOccured = Utility.WatchForEvents(watcher, WatcherChangeTypes.Deleted);
92+
9293
watcher.Path = Path.GetFullPath(dir.Path);
9394
watcher.Filter = "*";
9495
watcher.IncludeSubdirectories = true;
@@ -125,9 +126,10 @@ public static void FileSystemWatcher_Deleted_FileDeletedInNestedDirectory()
125126
{
126127
using (var dir = Utility.CreateTestDirectory())
127128
using (var watcher = new FileSystemWatcher())
128-
using (AutoResetEvent createOccured = Utility.WatchForEvents(watcher, WatcherChangeTypes.Created))
129-
using (AutoResetEvent eventOccured = Utility.WatchForEvents(watcher, WatcherChangeTypes.Deleted))
130129
{
130+
AutoResetEvent createOccured = Utility.WatchForEvents(watcher, WatcherChangeTypes.Created); // not "using" to avoid race conditions with FSW callbacks
131+
AutoResetEvent eventOccured = Utility.WatchForEvents(watcher, WatcherChangeTypes.Deleted);
132+
131133
watcher.Path = Path.GetFullPath(dir.Path);
132134
watcher.Filter = "*";
133135
watcher.IncludeSubdirectories = true;

src/System.IO.FileSystem.Watcher/tests/Utility/Utility.cs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,15 +102,16 @@ public static void ExpectNoEvent(WaitHandle eventOccured, string eventName, int
102102
}
103103

104104
public static void TestNestedDirectoriesHelper(
105-
WatcherChangeTypes change,
106-
Action<AutoResetEvent, TemporaryTestDirectory> action,
107-
NotifyFilters changeFilers = NotifyFilters.LastWrite | NotifyFilters.FileName | NotifyFilters.DirectoryName)
105+
WatcherChangeTypes change,
106+
Action<AutoResetEvent, TemporaryTestDirectory> action,
107+
NotifyFilters changeFilers = NotifyFilters.LastWrite | NotifyFilters.FileName | NotifyFilters.DirectoryName)
108108
{
109109
using (var dir = Utility.CreateTestDirectory())
110110
using (var watcher = new FileSystemWatcher())
111-
using (AutoResetEvent createdOccured = Utility.WatchForEvents(watcher, WatcherChangeTypes.Created))
112-
using (AutoResetEvent eventOccured = Utility.WatchForEvents(watcher, change))
113111
{
112+
AutoResetEvent createdOccured = Utility.WatchForEvents(watcher, WatcherChangeTypes.Created); // not "using" to avoid race conditions with FSW callbacks
113+
AutoResetEvent eventOccured = Utility.WatchForEvents(watcher, change);
114+
114115
watcher.Path = Path.GetFullPath(dir.Path);
115116
watcher.Filter = "*";
116117
watcher.NotifyFilter = changeFilers;

0 commit comments

Comments
 (0)