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

Commit 750602b

Browse files
author
Jonathan Miller
committed
Adding new Unit Tests to the FileSystemWatcher code base and fixing a
small bug in the Linux FileSystemWatcher that would notify a listener about a new directory before we had a chance to process it, which could lead to missed notifications and not fit user expectations.
1 parent 5487747 commit 750602b

File tree

7 files changed

+205
-17
lines changed

7 files changed

+205
-17
lines changed

‎src/Common/src/Interop/Linux/libc/Interop.inotify.cs

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,30 @@ internal static partial class libc
1414
[DllImport(Libraries.Libc, SetLastError = true)]
1515
internal static extern int inotify_add_watch(int fd, string pathname, uint mask);
1616

17-
[DllImport(Libraries.Libc, SetLastError = true)]
18-
internal static extern int inotify_rm_watch(int fd, int wd);
17+
[DllImport(Libraries.Libc, SetLastError = true, EntryPoint = "inotify_rm_watch")]
18+
private static extern int inotify_rm_watch_extern(int fd, int wd);
19+
20+
internal static int inotify_rm_watch(int fd, int wd)
21+
{
22+
int result = Interop.libc.inotify_rm_watch_extern(fd, wd);
23+
if (result < 0)
24+
{
25+
int hr = System.Runtime.InteropServices.Marshal.GetLastWin32Error();
26+
if (hr == Interop.Errors.EINVAL)
27+
{
28+
// This specific case means that there was a deleted event in the queue that was not processed
29+
// so this call is expected to fail since the WatchDescriptor is no longer valid and was cleaned
30+
// up automatically by the OS.
31+
result = 0;
32+
}
33+
else
34+
{
35+
System.Diagnostics.Debug.Fail("inotify_rm_watch failed with " + hr);
36+
}
37+
}
38+
39+
return result;
40+
}
1941

2042
[Flags]
2143
internal enum NotifyEvents

‎src/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Linux.cs

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010

1111
namespace System.IO
1212
{
13+
// Note: This class has an OS Limitation where the inotify API can miss events if a directory is created and immediately has
14+
// changes underneath. This is due to the inotify* APIs not being recursive and needing to call inotify_add_watch on
15+
// each subdirectory, causing a race between adding the watch and file system events happening.
1316
public partial class FileSystemWatcher
1417
{
1518
/// <summary>Starts a new watch operation if one is not currently running.</summary>
@@ -166,7 +169,7 @@ private static Interop.libc.NotifyEvents TranslateFilters(NotifyFilters filters)
166169
}
167170

168171
/// <summary>
169-
/// State and processing associatd with an active watch operation. This state is kept separate from FileSystemWatcher to avoid
172+
/// State and processing associated with an active watch operation. This state is kept separate from FileSystemWatcher to avoid
170173
/// race conditions when a user starts/stops/starts/stops/etc. in quick succession, resulting in the potential for multiple
171174
/// active operations. It also helps with avoiding rooted cycles and enabling proper finalization.
172175
/// </summary>
@@ -470,7 +473,6 @@ private void ProcessEvents()
470473
}
471474

472475
uint mask = nextEvent.mask;
473-
bool addWatch = false;
474476
string expandedName = null;
475477
WatchedDirectory associatedDirectoryEntry = null;
476478

@@ -531,6 +533,15 @@ private void ProcessEvents()
531533
previousEventCookie = 0;
532534
}
533535

536+
// If the event signaled that there's a new subdirectory and if we're monitoring subdirectories,
537+
// add a watch for it.
538+
const Interop.libc.NotifyEvents AddMaskFilters = Interop.libc.NotifyEvents.IN_CREATE | Interop.libc.NotifyEvents.IN_MOVED_TO;
539+
bool addWatch = ((mask & (uint)AddMaskFilters) != 0);
540+
if (addWatch && isDir && _includeSubdirectories)
541+
{
542+
AddDirectoryWatch(associatedDirectoryEntry, nextEvent.name);
543+
}
544+
534545
const Interop.libc.NotifyEvents switchMask =
535546
Interop.libc.NotifyEvents.IN_Q_OVERFLOW | Interop.libc.NotifyEvents.IN_IGNORED |
536547
Interop.libc.NotifyEvents.IN_CREATE | Interop.libc.NotifyEvents.IN_DELETE |
@@ -543,7 +554,6 @@ private void ProcessEvents()
543554
break;
544555
case Interop.libc.NotifyEvents.IN_CREATE:
545556
watcher.NotifyFileSystemEventArgs(WatcherChangeTypes.Created, expandedName);
546-
addWatch = true;
547557
break;
548558
case Interop.libc.NotifyEvents.IN_IGNORED:
549559
// We're getting an IN_IGNORED because a directory watch was removed.
@@ -582,17 +592,9 @@ private void ProcessEvents()
582592
previousEventName = null;
583593
previousEventParent = null;
584594
previousEventCookie = 0;
585-
addWatch = true; // for either rename or creation, we need to update our state
586595
break;
587596
}
588597

589-
// If the event signaled that there's a new subdirectory and if we're monitoring subdirectories,
590-
// add a watch for it.
591-
if (addWatch && isDir && _includeSubdirectories)
592-
{
593-
AddDirectoryWatch(associatedDirectoryEntry, nextEvent.name);
594-
}
595-
596598
// Drop our strong reference to the watcher now that we're potentially going to block again for another read
597599
watcher = null;
598600
}

‎src/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Changed.cs

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public static void FileSystemWatcher_Changed_Negative()
7777
}
7878

7979
[Fact, ActiveIssue(2279)]
80-
public static void FileSystemWatcher_ChangeWatchedFolder()
80+
public static void FileSystemWatcher_Changed_WatchedFolder()
8181
{
8282
using (var dir = Utility.CreateTestDirectory())
8383
using (var watcher = new FileSystemWatcher())
@@ -93,4 +93,29 @@ public static void FileSystemWatcher_ChangeWatchedFolder()
9393
Utility.ExpectEvent(eventOccured, "changed");
9494
}
9595
}
96+
97+
[Fact]
98+
public static void FileSystemWatcher_Changed_NestedDirectories()
99+
{
100+
Utility.TestNestedDirectoriesHelper(WatcherChangeTypes.Changed, (AutoResetEvent are, TemporaryTestDirectory ttd) =>
101+
{
102+
Directory.SetLastAccessTime(ttd.Path, DateTime.Now);
103+
Utility.ExpectEvent(are, "changed");
104+
},
105+
NotifyFilters.DirectoryName | NotifyFilters.LastAccess);
106+
}
107+
108+
[Fact]
109+
public static void FileSystemWatcher_Changed_FileInNestedDirectory()
110+
{
111+
Utility.TestNestedDirectoriesHelper(WatcherChangeTypes.Changed, (AutoResetEvent are, TemporaryTestDirectory ttd) =>
112+
{
113+
using (var nestedFile = new TemporaryTestFile(Path.Combine(ttd.Path, "nestedFile")))
114+
{
115+
Directory.SetLastAccessTime(nestedFile.Path, DateTime.Now);
116+
Utility.ExpectEvent(are, "changed");
117+
}
118+
},
119+
NotifyFilters.DirectoryName | NotifyFilters.LastAccess | NotifyFilters.FileName);
120+
}
96121
}

‎src/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Created.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,4 +111,16 @@ public static void FileSystemWatcher_Created_Negative()
111111
Utility.ExpectNoEvent(eventOccured, "created");
112112
}
113113
}
114+
115+
[Fact]
116+
public static void FileSystemWatcher_Created_FileCreatedInNestedDirectory()
117+
{
118+
Utility.TestNestedDirectoriesHelper(WatcherChangeTypes.Created, (AutoResetEvent are, TemporaryTestDirectory ttd) =>
119+
{
120+
using (var nestedFile = new TemporaryTestFile(Path.Combine(ttd.Path, "nestedFile")))
121+
{
122+
Utility.ExpectEvent(are, "nested file created", 1000 * 30);
123+
}
124+
});
125+
}
114126
}

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

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,78 @@ public static void FileSystemWatcher_Deleted_Negative()
8080
}
8181
}
8282
}
83+
84+
[Fact]
85+
public static void FileSystemWatcher_Deleted_NestedDirectories()
86+
{
87+
using (var dir = Utility.CreateTestDirectory())
88+
using (var watcher = new FileSystemWatcher())
89+
using (AutoResetEvent createOccured = Utility.WatchForEvents(watcher, WatcherChangeTypes.Created))
90+
using (AutoResetEvent eventOccured = Utility.WatchForEvents(watcher, WatcherChangeTypes.Deleted))
91+
{
92+
watcher.Path = Path.GetFullPath(dir.Path);
93+
watcher.Filter = "*";
94+
watcher.IncludeSubdirectories = true;
95+
watcher.EnableRaisingEvents = true;
96+
97+
using (var firstDir = new TemporaryTestDirectory(Path.Combine(dir.Path, "dir1")))
98+
{
99+
// Wait for the created event
100+
Utility.ExpectEvent(createOccured, "create", 1000 * 30);
101+
102+
using (var secondDir = new TemporaryTestDirectory(Path.Combine(firstDir.Path, "dir2")))
103+
{
104+
// Wait for the created event
105+
Utility.ExpectEvent(createOccured, "create", 1000 * 30);
106+
107+
using (var nestedDir = new TemporaryTestDirectory(Path.Combine(secondDir.Path, "nested")))
108+
{
109+
// Wait for the created event
110+
Utility.ExpectEvent(createOccured, "create", 1000 * 30);
111+
}
112+
113+
Utility.ExpectEvent(eventOccured, "deleted");
114+
}
115+
116+
Utility.ExpectEvent(eventOccured, "deleted");
117+
}
118+
119+
Utility.ExpectEvent(eventOccured, "deleted");
120+
}
121+
}
122+
123+
[Fact]
124+
public static void FileSystemWatcher_Deleted_FileDeletedInNestedDirectory()
125+
{
126+
using (var dir = Utility.CreateTestDirectory())
127+
using (var watcher = new FileSystemWatcher())
128+
using (AutoResetEvent createOccured = Utility.WatchForEvents(watcher, WatcherChangeTypes.Created))
129+
using (AutoResetEvent eventOccured = Utility.WatchForEvents(watcher, WatcherChangeTypes.Deleted))
130+
{
131+
watcher.Path = Path.GetFullPath(dir.Path);
132+
watcher.Filter = "*";
133+
watcher.IncludeSubdirectories = true;
134+
watcher.EnableRaisingEvents = true;
135+
136+
using (var firstDir = new TemporaryTestDirectory(Path.Combine(dir.Path, "dir1")))
137+
{
138+
// Wait for the created event
139+
Utility.ExpectEvent(createOccured, "create", 1000 * 30);
140+
141+
using (var secondDir = new TemporaryTestDirectory(Path.Combine(dir.Path, "dir2")))
142+
{
143+
// Wait for the created event
144+
Utility.ExpectEvent(createOccured, "create", 1000 * 30);
145+
146+
using (var nestedDir = new TemporaryTestFile(Path.Combine(secondDir.Path, "nestedFile"))) { }
147+
148+
Utility.ExpectEvent(eventOccured, "deleted");
149+
}
150+
151+
Utility.ExpectEvent(eventOccured, "deleted");
152+
}
153+
154+
Utility.ExpectEvent(eventOccured, "deleted");
155+
}
156+
}
83157
}

‎src/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Renamed.cs

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) Microsoft. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

4-
using System;
54
using System.IO;
65
using System.Threading;
76
using Xunit;
@@ -79,4 +78,27 @@ public static void FileSystemWatcher_Renamed_Negative()
7978
Utility.ExpectNoEvent(eventOccured, "created");
8079
}
8180
}
81+
82+
[Fact]
83+
public static void FileSystemWatcher_Renamed_NestedDirectory()
84+
{
85+
Utility.TestNestedDirectoriesHelper(WatcherChangeTypes.Renamed, (AutoResetEvent are, TemporaryTestDirectory ttd) =>
86+
{
87+
ttd.Move(ttd.Path + "_2");
88+
Utility.ExpectEvent(are, "renamed");
89+
});
90+
}
91+
92+
[Fact]
93+
public static void FileSystemWatcher_Renamed_FileInNestedDirectory()
94+
{
95+
Utility.TestNestedDirectoriesHelper(WatcherChangeTypes.Renamed, (AutoResetEvent are, TemporaryTestDirectory ttd) =>
96+
{
97+
using (var nestedFile = new TemporaryTestFile(Path.Combine(ttd.Path, "nestedFile")))
98+
{
99+
nestedFile.Move(nestedFile.Path + "_2");
100+
Utility.ExpectEvent(are, "renamed");
101+
}
102+
});
103+
}
82104
}

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

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ public static class Utility
1212
// events are reported asynchronously by the OS, so allow an amount of time for
1313
// them to arrive before testing an assertion.
1414
public const int Timeout = 500;
15+
public const int WaitForCreationTimeoutInMs = 1000 * 30;
1516

1617
public static TemporaryTestFile CreateTestFile([CallerMemberName] string path = null)
1718
{
@@ -91,12 +92,42 @@ public static AutoResetEvent WatchForEvents(FileSystemWatcher watcher, WatcherCh
9192
public static void ExpectEvent(WaitHandle eventOccured, string eventName, int timeout = Utility.Timeout)
9293
{
9394
string message = String.Format("Didn't observe a {0} event within {1}ms", eventName, timeout);
94-
Assert.True(eventOccured.WaitOne(Utility.Timeout), message);
95+
Assert.True(eventOccured.WaitOne(timeout), message);
9596
}
9697

9798
public static void ExpectNoEvent(WaitHandle eventOccured, string eventName, int timeout = Utility.Timeout)
9899
{
99100
string message = String.Format("Should not observe a {0} event", eventName);
100-
Assert.False(eventOccured.WaitOne(Utility.Timeout), message);
101+
Assert.False(eventOccured.WaitOne(timeout), message);
102+
}
103+
104+
public static void TestNestedDirectoriesHelper(
105+
WatcherChangeTypes change,
106+
Action<AutoResetEvent, TemporaryTestDirectory> action,
107+
NotifyFilters changeFilers = NotifyFilters.LastWrite | NotifyFilters.FileName | NotifyFilters.DirectoryName)
108+
{
109+
using (var dir = Utility.CreateTestDirectory())
110+
using (var watcher = new FileSystemWatcher())
111+
using (AutoResetEvent createdOccured = Utility.WatchForEvents(watcher, WatcherChangeTypes.Created))
112+
using (AutoResetEvent eventOccured = Utility.WatchForEvents(watcher, change))
113+
{
114+
watcher.Path = Path.GetFullPath(dir.Path);
115+
watcher.Filter = "*";
116+
watcher.NotifyFilter = changeFilers;
117+
watcher.IncludeSubdirectories = true;
118+
watcher.EnableRaisingEvents = true;
119+
120+
using (var firstDir = new TemporaryTestDirectory(Path.Combine(dir.Path, "dir1")))
121+
{
122+
Utility.ExpectEvent(createdOccured, "dir1 created", WaitForCreationTimeoutInMs);
123+
124+
using (var nestedDir = new TemporaryTestDirectory(Path.Combine(firstDir.Path, "nested")))
125+
{
126+
Utility.ExpectEvent(createdOccured, "nested created", WaitForCreationTimeoutInMs);
127+
128+
action(eventOccured, nestedDir);
129+
}
130+
}
131+
}
101132
}
102133
}

0 commit comments

Comments
 (0)