Fix invoking of FileSystemWatcher (#53151)

Call `protected` `On...` event raising methods from `Notify...` methods, to invoke `SynchronizingObject` when required. Fix #52644
dotnet · Aug 13, 2021 · 5817dc8 · 5817dc8
1 parent 42af636
commit 5817dc8
Show file tree

Hide file tree

Showing 12 changed files with 257 additions and 57 deletions.
diff --git a/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.cs b/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.cs
@@ -408,8 +408,11 @@ private bool MatchPattern(ReadOnlySpan<char> relativePath)
         /// </summary>
         private void NotifyInternalBufferOverflowEvent()
         {
-            _onErrorHandler?.Invoke(this, new ErrorEventArgs(
-                    new InternalBufferOverflowException(SR.Format(SR.FSW_BufferOverflow, _directory))));
+            if (_onErrorHandler != null)
+            {
+                OnError(new ErrorEventArgs(
+                        new InternalBufferOverflowException(SR.Format(SR.FSW_BufferOverflow, _directory))));
+            }
         }
 
         /// <summary>
@@ -418,11 +421,10 @@ private void NotifyInternalBufferOverflowEvent()
         private void NotifyRenameEventArgs(WatcherChangeTypes action, ReadOnlySpan<char> name, ReadOnlySpan<char> oldName)
         {
             // filter if there's no handler or neither new name or old name match a specified pattern
-            RenamedEventHandler? handler = _onRenamedHandler;
-            if (handler != null &&
+            if (_onRenamedHandler != null &&
                 (MatchPattern(name) || MatchPattern(oldName)))
             {
-                handler(this, new RenamedEventArgs(action, _directory, name.IsEmpty ? null : name.ToString(), oldName.IsEmpty ? null : oldName.ToString()));
+                OnRenamed(new RenamedEventArgs(action, _directory, name.IsEmpty ? null : name.ToString(), oldName.IsEmpty ? null : oldName.ToString()));
             }
         }
 
@@ -451,7 +453,7 @@ private void NotifyFileSystemEventArgs(WatcherChangeTypes changeType, ReadOnlySp
 
             if (handler != null && MatchPattern(name.IsEmpty ? _directory : name))
             {
-                handler(this, new FileSystemEventArgs(changeType, _directory, name.IsEmpty ? null : name.ToString()));
+                InvokeOn(new FileSystemEventArgs(changeType, _directory, name.IsEmpty ? null : name.ToString()), handler);
             }
         }
 
@@ -464,7 +466,7 @@ private void NotifyFileSystemEventArgs(WatcherChangeTypes changeType, string nam
 
             if (handler != null && MatchPattern(string.IsNullOrEmpty(name) ? _directory : name))
             {
-                handler(this, new FileSystemEventArgs(changeType, _directory, name));
+                InvokeOn(new FileSystemEventArgs(changeType, _directory, name), handler);
             }
         }
 

diff --git a/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Directory.Changed.cs b/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Directory.Changed.cs
@@ -78,5 +78,22 @@ public void FileSystemWatcher_Directory_Changed_SymLink()
                 ExpectEvent(watcher, 0, action, cleanup, dir.Path);
             }
         }
+
+        [Fact]
+        public void FileSystemWatcher_Directory_Changed_SynchronizingObject()
+        {
+            using (var testDirectory = new TempDirectory(GetTestFilePath()))
+            using (var dir = new TempDirectory(Path.Combine(testDirectory.Path, "dir")))
+            using (var watcher = new FileSystemWatcher(testDirectory.Path, Path.GetFileName(dir.Path)))
+            {
+                TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
+                watcher.SynchronizingObject = invoker;
+
+                Action action = () => Directory.SetLastWriteTime(dir.Path, DateTime.Now + TimeSpan.FromSeconds(10));
+
+                ExpectEvent(watcher, WatcherChangeTypes.Changed, action, expectedPath: dir.Path);
+                Assert.True(invoker.BeginInvoke_Called);
+            }
+        }
     }
 }
diff --git a/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Directory.Create.cs b/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Directory.Create.cs
@@ -102,5 +102,25 @@ public void FileSystemWatcher_Directory_Create_SymLink()
                 ExpectEvent(watcher, WatcherChangeTypes.Created, action, cleanup, symLinkPath);
             }
         }
+
+        [Fact]
+        public void FileSystemWatcher_Directory_Create_SynchronizingObject()
+        {
+            using (var testDirectory = new TempDirectory(GetTestFilePath()))
+            using (var watcher = new FileSystemWatcher(testDirectory.Path))
+            {
+                TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
+                watcher.SynchronizingObject = invoker;
+
+                string dirName = Path.Combine(testDirectory.Path, "dir");
+                watcher.Filter = Path.GetFileName(dirName);
+
+                Action action = () => Directory.CreateDirectory(dirName);
+                Action cleanup = () => Directory.Delete(dirName);
+
+                ExpectEvent(watcher, WatcherChangeTypes.Created, action, cleanup, dirName);
+                Assert.True(invoker.BeginInvoke_Called);
+            }
+        }
     }
 }
diff --git a/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Directory.Delete.cs b/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Directory.Delete.cs
@@ -85,5 +85,26 @@ public void FileSystemWatcher_Directory_Delete_SymLink()
                 ExpectEvent(watcher, WatcherChangeTypes.Deleted, action, cleanup, expectedPath: symLinkPath);
             }
         }
+
+        [Fact]
+        public void FileSystemWatcher_Directory_Delete_SynchronizingObject()
+        {
+            using (var testDirectory = new TempDirectory(GetTestFilePath()))
+            using (var watcher = new FileSystemWatcher(testDirectory.Path))
+            {
+                TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
+                watcher.SynchronizingObject = invoker;
+
+                string dirName = Path.Combine(testDirectory.Path, "dir");
+                watcher.Filter = Path.GetFileName(dirName);
+
+                Action action = () => Directory.Delete(dirName);
+                Action cleanup = () => Directory.CreateDirectory(dirName);
+                cleanup();
+
+                ExpectEvent(watcher, WatcherChangeTypes.Deleted, action, cleanup, dirName);
+                Assert.True(invoker.BeginInvoke_Called);
+            }
+        }
     }
 }
diff --git a/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Directory.Move.cs b/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Directory.Move.cs
@@ -93,6 +93,27 @@ public void Directory_Move_With_Set_NotifyFilter()
             DirectoryMove_WithNotifyFilter(WatcherChangeTypes.Renamed);
         }
 
+        [Fact]
+        public void Directory_Move_SynchronizingObject()
+        {
+            using (var testDirectory = new TempDirectory(GetTestFilePath()))
+            using (var dir = new TempDirectory(Path.Combine(testDirectory.Path, "dir")))
+            using (var watcher = new FileSystemWatcher(testDirectory.Path))
+            {
+                TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
+                watcher.SynchronizingObject = invoker;
+
+                string sourcePath = dir.Path;
+                string targetPath = Path.Combine(testDirectory.Path, "target");
+
+                Action action = () => Directory.Move(sourcePath, targetPath);
+                Action cleanup = () => Directory.Move(targetPath, sourcePath);
+
+                ExpectEvent(watcher, WatcherChangeTypes.Renamed, action, cleanup, targetPath);
+                Assert.True(invoker.BeginInvoke_Called);
+            }
+        }
+
         #region Test Helpers
 
         private void DirectoryMove_SameDirectory(WatcherChangeTypes eventType)

diff --git a/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.File.Changed.cs b/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.File.Changed.cs
@@ -82,5 +82,22 @@ public void FileSystemWatcher_File_Changed_SymLink()
                 ExpectEvent(watcher, 0, action, cleanup, expectedPath: file.Path);
             }
         }
+
+        [Fact]
+        public void FileSystemWatcher_File_Changed_SynchronizingObject()
+        {
+            using (var testDirectory = new TempDirectory(GetTestFilePath()))
+            using (var file = new TempFile(Path.Combine(testDirectory.Path, "file")))
+            using (var watcher = new FileSystemWatcher(testDirectory.Path, Path.GetFileName(file.Path)))
+            {
+                TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
+                watcher.SynchronizingObject = invoker;
+
+                Action action = () => Directory.SetLastWriteTime(file.Path, DateTime.Now + TimeSpan.FromSeconds(10));
+
+                ExpectEvent(watcher, WatcherChangeTypes.Changed, action, expectedPath: file.Path);
+                Assert.True(invoker.BeginInvoke_Called);
+            }
+        }
     }
 }
diff --git a/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.File.Create.cs b/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.File.Create.cs
@@ -141,5 +141,25 @@ public void FileSystemWatcher_File_Create_SymLink()
                 ExpectEvent(watcher, WatcherChangeTypes.Created, action, cleanup, symLinkPath);
             }
         }
+
+        [Fact]
+        public void FileSystemWatcher_File_Create_SynchronizingObject()
+        {
+            using (var testDirectory = new TempDirectory(GetTestFilePath()))
+            using (var watcher = new FileSystemWatcher(testDirectory.Path))
+            {
+                TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
+                watcher.SynchronizingObject = invoker;
+
+                string fileName = Path.Combine(testDirectory.Path, "file");
+                watcher.Filter = Path.GetFileName(fileName);
+
+                Action action = () => File.Create(fileName).Dispose();
+                Action cleanup = () => File.Delete(fileName);
+
+                ExpectEvent(watcher, WatcherChangeTypes.Created, action, cleanup, fileName);
+                Assert.True(invoker.BeginInvoke_Called);
+            }
+        }
     }
 }
diff --git a/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.File.Delete.cs b/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.File.Delete.cs
@@ -106,5 +106,26 @@ public void FileSystemWatcher_File_Delete_SymLink()
                 ExpectEvent(watcher, WatcherChangeTypes.Deleted, action, cleanup, symLinkPath);
             }
         }
+
+        [Fact]
+        public void FileSystemWatcher_File_Delete_SynchronizingObject()
+        {
+            using (var testDirectory = new TempDirectory(GetTestFilePath()))
+            using (var watcher = new FileSystemWatcher(testDirectory.Path))
+            {
+                TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
+                watcher.SynchronizingObject = invoker;
+
+                string fileName = Path.Combine(testDirectory.Path, "file");
+                watcher.Filter = Path.GetFileName(fileName);
+
+                Action action = () => File.Delete(fileName);
+                Action cleanup = () => File.Create(fileName).Dispose();
+                cleanup();
+
+                ExpectEvent(watcher, WatcherChangeTypes.Deleted, action, cleanup, fileName);
+                Assert.True(invoker.BeginInvoke_Called);
+            }
+        }
     }
 }
diff --git a/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.File.Move.cs b/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.File.Move.cs
@@ -121,6 +121,29 @@ public void Unix_File_Move_With_Set_NotifyFilter()
             FileMove_WithNotifyFilter(WatcherChangeTypes.Renamed);
         }
 
+        [Fact]
+        public void File_Move_SynchronizingObject()
+        {
+            using (var testDirectory = new TempDirectory(GetTestFilePath()))
+            using (var dir = new TempDirectory(Path.Combine(testDirectory.Path, "dir")))
+            using (var testFile = new TempFile(Path.Combine(dir.Path, "file")))
+            using (var watcher = new FileSystemWatcher(dir.Path, "*"))
+            {
+                TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
+                watcher.SynchronizingObject = invoker;
+
+                string sourcePath = testFile.Path;
+                string targetPath = testFile.Path + "_Renamed";
+
+                // Move the testFile to a different name in the same directory
+                Action action = () => File.Move(sourcePath, targetPath);
+                Action cleanup = () => File.Move(targetPath, sourcePath);
+
+                ExpectEvent(watcher, WatcherChangeTypes.Renamed, action, cleanup, targetPath);
+                Assert.True(invoker.BeginInvoke_Called);
+            }
+        }
+
         #region Test Helpers
 
         private void FileMove_SameDirectory(WatcherChangeTypes eventType)

diff --git a/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.InternalBufferSize.cs b/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.InternalBufferSize.cs
@@ -44,40 +44,80 @@ public class InternalBufferSizeTests : FileSystemWatcherTest
         [PlatformSpecific(TestPlatforms.Windows)]  // Uses P/Invokes
         public void FileSystemWatcher_InternalBufferSize(bool setToHigherCapacity)
         {
+            ManualResetEvent unblockHandler = new ManualResetEvent(false);
             using (var testDirectory = new TempDirectory(GetTestFilePath()))
             using (var file = new TempFile(Path.Combine(testDirectory.Path, "file")))
-            using (var watcher = new FileSystemWatcher(testDirectory.Path, Path.GetFileName(file.Path)))
+            using (FileSystemWatcher watcher = CreateWatcher(testDirectory.Path, file.Path, unblockHandler))
             {
-                int internalBufferOperationCapacity = watcher.InternalBufferSize / (17 + Path.GetFileName(file.Path).Length);
+                int internalBufferOperationCapacity = CalculateInternalBufferOperationCapacity(watcher.InternalBufferSize, file.Path);
 
                 // Set the capacity high to ensure no error events arise.
                 if (setToHigherCapacity)
                     watcher.InternalBufferSize = watcher.InternalBufferSize * 12;
 
-                // block the handling thread
-                ManualResetEvent unblockHandler = new ManualResetEvent(false);
-                watcher.Changed += (o, e) =>
-                {
-                    unblockHandler.WaitOne();
-                };
-
-                // generate enough file change events to overflow the default buffer
-                Action action = () =>
-                {
-                    for (int i = 1; i < internalBufferOperationCapacity * 10; i++)
-                    {
-                        File.SetLastWriteTime(file.Path, DateTime.Now + TimeSpan.FromSeconds(i));
-                    }
-
-                    unblockHandler.Set();
-                };
-                Action cleanup = () => unblockHandler.Reset();
+                Action action = GetAction(unblockHandler, internalBufferOperationCapacity, file.Path);
+                Action cleanup = GetCleanup(unblockHandler);
 
                 if (setToHigherCapacity)
                     ExpectNoError(watcher, action, cleanup);
                 else
                     ExpectError(watcher, action, cleanup);
             }
         }
+
+        [Fact]
+        [PlatformSpecific(TestPlatforms.Windows)]
+        public void FileSystemWatcher_InternalBufferSize_SynchronizingObject()
+        {
+            ManualResetEvent unblockHandler = new ManualResetEvent(false);
+            using (var testDirectory = new TempDirectory(GetTestFilePath()))
+            using (var file = new TempFile(Path.Combine(testDirectory.Path, "file")))
+            using (FileSystemWatcher watcher = CreateWatcher(testDirectory.Path, file.Path, unblockHandler))
+            {
+                TestISynchronizeInvoke invoker = new TestISynchronizeInvoke();
+                watcher.SynchronizingObject = invoker;
+
+                int internalBufferOperationCapacity = CalculateInternalBufferOperationCapacity(watcher.InternalBufferSize, file.Path);
+
+                Action action = GetAction(unblockHandler, internalBufferOperationCapacity, file.Path);
+                Action cleanup = GetCleanup(unblockHandler);
+
+                ExpectError(watcher, action, cleanup);
+                Assert.True(invoker.BeginInvoke_Called);
+            }
+        }
+
+        #region Test Helpers
+
+        private FileSystemWatcher CreateWatcher(string testDirectoryPath, string filePath, ManualResetEvent unblockHandler)
+        {
+            var watcher = new FileSystemWatcher(testDirectoryPath, Path.GetFileName(filePath));
+
+            // block the handling thread
+            watcher.Changed += (o, e) => unblockHandler.WaitOne();
+
+            return watcher;
+        }
+
+        private int CalculateInternalBufferOperationCapacity(int internalBufferSize, string filePath) =>
+            internalBufferSize / (17 + Path.GetFileName(filePath).Length);
+
+        private Action GetAction(ManualResetEvent unblockHandler, int internalBufferOperationCapacity, string filePath)
+        {
+            return () =>
+            {
+                // generate enough file change events to overflow the default buffer
+                for (int i = 1; i < internalBufferOperationCapacity * 10; i++)
+                {
+                    File.SetLastWriteTime(filePath, DateTime.Now + TimeSpan.FromSeconds(i));
+                }
+
+                unblockHandler.Set();
+            };
+        }
+
+        private Action GetCleanup(ManualResetEvent unblockHandler) => () => unblockHandler.Reset();
+
+        #endregion
     }
 }