Skip to content

Commit d8706a1

Browse files
thomsjstephentoub
thomsj
authored and
stephentoub
committed
Fix invoking of FileSystemWatcher
Call `protected` `On...` event raising methods from `Notify...` methods, to invoke `SynchronizingObject` when required. Fix #52644
1 parent 134c25d commit d8706a1

12 files changed

+257
-57
lines changed

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -408,8 +408,11 @@ private bool MatchPattern(ReadOnlySpan<char> relativePath)
408408
/// </summary>
409409
private void NotifyInternalBufferOverflowEvent()
410410
{
411-
_onErrorHandler?.Invoke(this, new ErrorEventArgs(
412-
new InternalBufferOverflowException(SR.Format(SR.FSW_BufferOverflow, _directory))));
411+
if (_onErrorHandler != null)
412+
{
413+
OnError(new ErrorEventArgs(
414+
new InternalBufferOverflowException(SR.Format(SR.FSW_BufferOverflow, _directory))));
415+
}
413416
}
414417

415418
/// <summary>
@@ -418,11 +421,10 @@ private void NotifyInternalBufferOverflowEvent()
418421
private void NotifyRenameEventArgs(WatcherChangeTypes action, ReadOnlySpan<char> name, ReadOnlySpan<char> oldName)
419422
{
420423
// filter if there's no handler or neither new name or old name match a specified pattern
421-
RenamedEventHandler? handler = _onRenamedHandler;
422-
if (handler != null &&
424+
if (_onRenamedHandler != null &&
423425
(MatchPattern(name) || MatchPattern(oldName)))
424426
{
425-
handler(this, new RenamedEventArgs(action, _directory, name.IsEmpty ? null : name.ToString(), oldName.IsEmpty ? null : oldName.ToString()));
427+
OnRenamed(new RenamedEventArgs(action, _directory, name.IsEmpty ? null : name.ToString(), oldName.IsEmpty ? null : oldName.ToString()));
426428
}
427429
}
428430

@@ -451,7 +453,7 @@ private void NotifyFileSystemEventArgs(WatcherChangeTypes changeType, ReadOnlySp
451453

452454
if (handler != null && MatchPattern(name.IsEmpty ? _directory : name))
453455
{
454-
handler(this, new FileSystemEventArgs(changeType, _directory, name.IsEmpty ? null : name.ToString()));
456+
InvokeOn(new FileSystemEventArgs(changeType, _directory, name.IsEmpty ? null : name.ToString()), handler);
455457
}
456458
}
457459

@@ -464,7 +466,7 @@ private void NotifyFileSystemEventArgs(WatcherChangeTypes changeType, string nam
464466

465467
if (handler != null && MatchPattern(string.IsNullOrEmpty(name) ? _directory : name))
466468
{
467-
handler(this, new FileSystemEventArgs(changeType, _directory, name));
469+
InvokeOn(new FileSystemEventArgs(changeType, _directory, name), handler);
468470
}
469471
}
470472

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,5 +78,22 @@ public void FileSystemWatcher_Directory_Changed_SymLink()
7878
ExpectEvent(watcher, 0, action, cleanup, dir.Path);
7979
}
8080
}
81+
82+
[Fact]
83+
public void FileSystemWatcher_Directory_Changed_SynchronizingObject()
84+
{
85+
using (var testDirectory = new TempDirectory(GetTestFilePath()))
86+
using (var dir = new TempDirectory(Path.Combine(testDirectory.Path, "dir")))
87+
using (var watcher = new FileSystemWatcher(testDirectory.Path, Path.GetFileName(dir.Path)))
88+
{
89+
TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
90+
watcher.SynchronizingObject = invoker;
91+
92+
Action action = () => Directory.SetLastWriteTime(dir.Path, DateTime.Now + TimeSpan.FromSeconds(10));
93+
94+
ExpectEvent(watcher, WatcherChangeTypes.Changed, action, expectedPath: dir.Path);
95+
Assert.True(invoker.BeginInvoke_Called);
96+
}
97+
}
8198
}
8299
}

‎src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Directory.Create.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,5 +102,25 @@ public void FileSystemWatcher_Directory_Create_SymLink()
102102
ExpectEvent(watcher, WatcherChangeTypes.Created, action, cleanup, symLinkPath);
103103
}
104104
}
105+
106+
[Fact]
107+
public void FileSystemWatcher_Directory_Create_SynchronizingObject()
108+
{
109+
using (var testDirectory = new TempDirectory(GetTestFilePath()))
110+
using (var watcher = new FileSystemWatcher(testDirectory.Path))
111+
{
112+
TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
113+
watcher.SynchronizingObject = invoker;
114+
115+
string dirName = Path.Combine(testDirectory.Path, "dir");
116+
watcher.Filter = Path.GetFileName(dirName);
117+
118+
Action action = () => Directory.CreateDirectory(dirName);
119+
Action cleanup = () => Directory.Delete(dirName);
120+
121+
ExpectEvent(watcher, WatcherChangeTypes.Created, action, cleanup, dirName);
122+
Assert.True(invoker.BeginInvoke_Called);
123+
}
124+
}
105125
}
106126
}

‎src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Directory.Delete.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,5 +85,26 @@ public void FileSystemWatcher_Directory_Delete_SymLink()
8585
ExpectEvent(watcher, WatcherChangeTypes.Deleted, action, cleanup, expectedPath: symLinkPath);
8686
}
8787
}
88+
89+
[Fact]
90+
public void FileSystemWatcher_Directory_Delete_SynchronizingObject()
91+
{
92+
using (var testDirectory = new TempDirectory(GetTestFilePath()))
93+
using (var watcher = new FileSystemWatcher(testDirectory.Path))
94+
{
95+
TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
96+
watcher.SynchronizingObject = invoker;
97+
98+
string dirName = Path.Combine(testDirectory.Path, "dir");
99+
watcher.Filter = Path.GetFileName(dirName);
100+
101+
Action action = () => Directory.Delete(dirName);
102+
Action cleanup = () => Directory.CreateDirectory(dirName);
103+
cleanup();
104+
105+
ExpectEvent(watcher, WatcherChangeTypes.Deleted, action, cleanup, dirName);
106+
Assert.True(invoker.BeginInvoke_Called);
107+
}
108+
}
88109
}
89110
}

‎src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Directory.Move.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,27 @@ public void Directory_Move_With_Set_NotifyFilter()
9393
DirectoryMove_WithNotifyFilter(WatcherChangeTypes.Renamed);
9494
}
9595

96+
[Fact]
97+
public void Directory_Move_SynchronizingObject()
98+
{
99+
using (var testDirectory = new TempDirectory(GetTestFilePath()))
100+
using (var dir = new TempDirectory(Path.Combine(testDirectory.Path, "dir")))
101+
using (var watcher = new FileSystemWatcher(testDirectory.Path))
102+
{
103+
TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
104+
watcher.SynchronizingObject = invoker;
105+
106+
string sourcePath = dir.Path;
107+
string targetPath = Path.Combine(testDirectory.Path, "target");
108+
109+
Action action = () => Directory.Move(sourcePath, targetPath);
110+
Action cleanup = () => Directory.Move(targetPath, sourcePath);
111+
112+
ExpectEvent(watcher, WatcherChangeTypes.Renamed, action, cleanup, targetPath);
113+
Assert.True(invoker.BeginInvoke_Called);
114+
}
115+
}
116+
96117
#region Test Helpers
97118

98119
private void DirectoryMove_SameDirectory(WatcherChangeTypes eventType)

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,5 +82,22 @@ public void FileSystemWatcher_File_Changed_SymLink()
8282
ExpectEvent(watcher, 0, action, cleanup, expectedPath: file.Path);
8383
}
8484
}
85+
86+
[Fact]
87+
public void FileSystemWatcher_File_Changed_SynchronizingObject()
88+
{
89+
using (var testDirectory = new TempDirectory(GetTestFilePath()))
90+
using (var file = new TempFile(Path.Combine(testDirectory.Path, "file")))
91+
using (var watcher = new FileSystemWatcher(testDirectory.Path, Path.GetFileName(file.Path)))
92+
{
93+
TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
94+
watcher.SynchronizingObject = invoker;
95+
96+
Action action = () => Directory.SetLastWriteTime(file.Path, DateTime.Now + TimeSpan.FromSeconds(10));
97+
98+
ExpectEvent(watcher, WatcherChangeTypes.Changed, action, expectedPath: file.Path);
99+
Assert.True(invoker.BeginInvoke_Called);
100+
}
101+
}
85102
}
86103
}

‎src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.File.Create.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,5 +141,25 @@ public void FileSystemWatcher_File_Create_SymLink()
141141
ExpectEvent(watcher, WatcherChangeTypes.Created, action, cleanup, symLinkPath);
142142
}
143143
}
144+
145+
[Fact]
146+
public void FileSystemWatcher_File_Create_SynchronizingObject()
147+
{
148+
using (var testDirectory = new TempDirectory(GetTestFilePath()))
149+
using (var watcher = new FileSystemWatcher(testDirectory.Path))
150+
{
151+
TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
152+
watcher.SynchronizingObject = invoker;
153+
154+
string fileName = Path.Combine(testDirectory.Path, "file");
155+
watcher.Filter = Path.GetFileName(fileName);
156+
157+
Action action = () => File.Create(fileName).Dispose();
158+
Action cleanup = () => File.Delete(fileName);
159+
160+
ExpectEvent(watcher, WatcherChangeTypes.Created, action, cleanup, fileName);
161+
Assert.True(invoker.BeginInvoke_Called);
162+
}
163+
}
144164
}
145165
}

‎src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.File.Delete.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,5 +106,26 @@ public void FileSystemWatcher_File_Delete_SymLink()
106106
ExpectEvent(watcher, WatcherChangeTypes.Deleted, action, cleanup, symLinkPath);
107107
}
108108
}
109+
110+
[Fact]
111+
public void FileSystemWatcher_File_Delete_SynchronizingObject()
112+
{
113+
using (var testDirectory = new TempDirectory(GetTestFilePath()))
114+
using (var watcher = new FileSystemWatcher(testDirectory.Path))
115+
{
116+
TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
117+
watcher.SynchronizingObject = invoker;
118+
119+
string fileName = Path.Combine(testDirectory.Path, "file");
120+
watcher.Filter = Path.GetFileName(fileName);
121+
122+
Action action = () => File.Delete(fileName);
123+
Action cleanup = () => File.Create(fileName).Dispose();
124+
cleanup();
125+
126+
ExpectEvent(watcher, WatcherChangeTypes.Deleted, action, cleanup, fileName);
127+
Assert.True(invoker.BeginInvoke_Called);
128+
}
129+
}
109130
}
110131
}

‎src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.File.Move.cs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,29 @@ public void Unix_File_Move_With_Set_NotifyFilter()
121121
FileMove_WithNotifyFilter(WatcherChangeTypes.Renamed);
122122
}
123123

124+
[Fact]
125+
public void File_Move_SynchronizingObject()
126+
{
127+
using (var testDirectory = new TempDirectory(GetTestFilePath()))
128+
using (var dir = new TempDirectory(Path.Combine(testDirectory.Path, "dir")))
129+
using (var testFile = new TempFile(Path.Combine(dir.Path, "file")))
130+
using (var watcher = new FileSystemWatcher(dir.Path, "*"))
131+
{
132+
TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
133+
watcher.SynchronizingObject = invoker;
134+
135+
string sourcePath = testFile.Path;
136+
string targetPath = testFile.Path + "_Renamed";
137+
138+
// Move the testFile to a different name in the same directory
139+
Action action = () => File.Move(sourcePath, targetPath);
140+
Action cleanup = () => File.Move(targetPath, sourcePath);
141+
142+
ExpectEvent(watcher, WatcherChangeTypes.Renamed, action, cleanup, targetPath);
143+
Assert.True(invoker.BeginInvoke_Called);
144+
}
145+
}
146+
124147
#region Test Helpers
125148

126149
private void FileMove_SameDirectory(WatcherChangeTypes eventType)

‎src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.InternalBufferSize.cs

Lines changed: 60 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -44,40 +44,80 @@ public class InternalBufferSizeTests : FileSystemWatcherTest
4444
[PlatformSpecific(TestPlatforms.Windows)] // Uses P/Invokes
4545
public void FileSystemWatcher_InternalBufferSize(bool setToHigherCapacity)
4646
{
47+
ManualResetEvent unblockHandler = new ManualResetEvent(false);
4748
using (var testDirectory = new TempDirectory(GetTestFilePath()))
4849
using (var file = new TempFile(Path.Combine(testDirectory.Path, "file")))
49-
using (var watcher = new FileSystemWatcher(testDirectory.Path, Path.GetFileName(file.Path)))
50+
using (FileSystemWatcher watcher = CreateWatcher(testDirectory.Path, file.Path, unblockHandler))
5051
{
51-
int internalBufferOperationCapacity = watcher.InternalBufferSize / (17 + Path.GetFileName(file.Path).Length);
52+
int internalBufferOperationCapacity = CalculateInternalBufferOperationCapacity(watcher.InternalBufferSize, file.Path);
5253

5354
// Set the capacity high to ensure no error events arise.
5455
if (setToHigherCapacity)
5556
watcher.InternalBufferSize = watcher.InternalBufferSize * 12;
5657

57-
// block the handling thread
58-
ManualResetEvent unblockHandler = new ManualResetEvent(false);
59-
watcher.Changed += (o, e) =>
60-
{
61-
unblockHandler.WaitOne();
62-
};
63-
64-
// generate enough file change events to overflow the default buffer
65-
Action action = () =>
66-
{
67-
for (int i = 1; i < internalBufferOperationCapacity * 10; i++)
68-
{
69-
File.SetLastWriteTime(file.Path, DateTime.Now + TimeSpan.FromSeconds(i));
70-
}
71-
72-
unblockHandler.Set();
73-
};
74-
Action cleanup = () => unblockHandler.Reset();
58+
Action action = GetAction(unblockHandler, internalBufferOperationCapacity, file.Path);
59+
Action cleanup = GetCleanup(unblockHandler);
7560

7661
if (setToHigherCapacity)
7762
ExpectNoError(watcher, action, cleanup);
7863
else
7964
ExpectError(watcher, action, cleanup);
8065
}
8166
}
67+
68+
[Fact]
69+
[PlatformSpecific(TestPlatforms.Windows)]
70+
public void FileSystemWatcher_InternalBufferSize_SynchronizingObject()
71+
{
72+
ManualResetEvent unblockHandler = new ManualResetEvent(false);
73+
using (var testDirectory = new TempDirectory(GetTestFilePath()))
74+
using (var file = new TempFile(Path.Combine(testDirectory.Path, "file")))
75+
using (FileSystemWatcher watcher = CreateWatcher(testDirectory.Path, file.Path, unblockHandler))
76+
{
77+
TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
78+
watcher.SynchronizingObject = invoker;
79+
80+
int internalBufferOperationCapacity = CalculateInternalBufferOperationCapacity(watcher.InternalBufferSize, file.Path);
81+
82+
Action action = GetAction(unblockHandler, internalBufferOperationCapacity, file.Path);
83+
Action cleanup = GetCleanup(unblockHandler);
84+
85+
ExpectError(watcher, action, cleanup);
86+
Assert.True(invoker.BeginInvoke_Called);
87+
}
88+
}
89+
90+
#region Test Helpers
91+
92+
private FileSystemWatcher CreateWatcher(string testDirectoryPath, string filePath, ManualResetEvent unblockHandler)
93+
{
94+
var watcher = new FileSystemWatcher(testDirectoryPath, Path.GetFileName(filePath));
95+
96+
// block the handling thread
97+
watcher.Changed += (o, e) => unblockHandler.WaitOne();
98+
99+
return watcher;
100+
}
101+
102+
private int CalculateInternalBufferOperationCapacity(int internalBufferSize, string filePath) =>
103+
internalBufferSize / (17 + Path.GetFileName(filePath).Length);
104+
105+
private Action GetAction(ManualResetEvent unblockHandler, int internalBufferOperationCapacity, string filePath)
106+
{
107+
return () =>
108+
{
109+
// generate enough file change events to overflow the default buffer
110+
for (int i = 1; i < internalBufferOperationCapacity * 10; i++)
111+
{
112+
File.SetLastWriteTime(filePath, DateTime.Now + TimeSpan.FromSeconds(i));
113+
}
114+
115+
unblockHandler.Set();
116+
};
117+
}
118+
119+
private Action GetCleanup(ManualResetEvent unblockHandler) => () => unblockHandler.Reset();
120+
121+
#endregion
82122
}
83123
}

0 commit comments

Comments
 (0)