From 2ccd563c90a84efd7c1b1b190c0b5ad5cfaab101 Mon Sep 17 00:00:00 2001 From: Dan Moseley Date: Sun, 23 Jun 2024 14:42:44 -0600 Subject: [PATCH 1/3] Fix flaky FSW test --- .../FileSystemWatcher.InternalBufferSize.cs | 37 +++++++++++++------ 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.InternalBufferSize.cs b/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.InternalBufferSize.cs index b15e77232657b..bf5441a4fd189 100644 --- a/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.InternalBufferSize.cs +++ b/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.InternalBufferSize.cs @@ -1,7 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; using System.Threading; +using System.Threading.Tasks; using Xunit; namespace System.IO.Tests @@ -32,26 +34,31 @@ public class InternalBufferSizeTests : FileSystemWatcherTest // it's internal buffer (up to some limit). Our docs say that limit is 64KB but testing on Win8.1 // indicates that it is much higher than this: I could grow the buffer up to 128 MB and still see // that it had an effect. The size needed per operation is determined by the struct layout of - // FILE_NOTIFY_INFORMATION. This works out to 16 + 2 * (Path.GetFileName(file.Path).Length + 1) bytes, where filePath + // FILE_NOTIFY_INFORMATION. This works out to 12 + 2 * (Path.GetFileName(file.Path).Length + 1) bytes, where filePath // is the path to changed file relative to the path passed into ReadDirectoryChanges. // At some point we might decide to improve how FSW handles this at which point we'll need // a better test for Error (perhaps just a mock), but for now there is some value in forcing this limit. + private const int ExcessEventsMultiplier = 250; // 100 does not reliably trigger the error + [Theory] [InlineData(true)] [InlineData(false)] [PlatformSpecific(TestPlatforms.Windows)] // Uses P/Invokes + [OuterLoop("A little slow")] public void FileSystemWatcher_InternalBufferSize(bool setToHigherCapacity) { ManualResetEvent unblockHandler = new ManualResetEvent(false); string file = CreateTestFile(TestDirectory, "file"); using (FileSystemWatcher watcher = CreateWatcher(TestDirectory, file, unblockHandler)) { - int internalBufferOperationCapacity = CalculateInternalBufferOperationCapacity(watcher.InternalBufferSize, file); + int internalBufferOperationCapacity = watcher.InternalBufferSize / (12 + 2 * (Path.GetFileName(file).Length + 1)); - // Set the capacity high to ensure no error events arise. if (setToHigherCapacity) - watcher.InternalBufferSize = watcher.InternalBufferSize * 12; + { + // Set the capacity high to ensure no error events arise. + watcher.InternalBufferSize = watcher.InternalBufferSize * ExcessEventsMultiplier * 2; + } Action action = GetAction(unblockHandler, internalBufferOperationCapacity, file); Action cleanup = GetCleanup(unblockHandler); @@ -65,6 +72,7 @@ public void FileSystemWatcher_InternalBufferSize(bool setToHigherCapacity) [Fact] [PlatformSpecific(TestPlatforms.Windows)] + [OuterLoop("A little slow")] public void FileSystemWatcher_InternalBufferSize_SynchronizingObject() { ManualResetEvent unblockHandler = new ManualResetEvent(false); @@ -74,7 +82,7 @@ public void FileSystemWatcher_InternalBufferSize_SynchronizingObject() TestISynchronizeInvoke invoker = new TestISynchronizeInvoke(); watcher.SynchronizingObject = invoker; - int internalBufferOperationCapacity = CalculateInternalBufferOperationCapacity(watcher.InternalBufferSize, file); + int internalBufferOperationCapacity = watcher.InternalBufferSize / (12 + 2 * (Path.GetFileName(file).Length + 1)); Action action = GetAction(unblockHandler, internalBufferOperationCapacity, file); Action cleanup = GetCleanup(unblockHandler); @@ -96,23 +104,30 @@ private FileSystemWatcher CreateWatcher(string testDirectoryPath, string filePat 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++) + List tasks = new (); + + // Generate enough file change events to overflow the default buffer + // For speed, do this on multiple threads + for (int i = 1; i < internalBufferOperationCapacity * ExcessEventsMultiplier; i++) { - File.SetLastWriteTime(filePath, DateTime.Now + TimeSpan.FromSeconds(i)); + tasks.Add(Task.Run(() => + { + // Each sets to a different time to ensure it triggers an event + File.SetLastWriteTime(filePath, DateTime.Now + TimeSpan.FromSeconds(i)); + })); } + Task.WaitAll(tasks); + unblockHandler.Set(); }; } + private Action GetCleanup(ManualResetEvent unblockHandler) => () => unblockHandler.Reset(); #endregion From c80e4157620bc93d319e417417510064c8018448 Mon Sep 17 00:00:00 2001 From: Dan Moseley Date: Sun, 23 Jun 2024 14:54:04 -0600 Subject: [PATCH 2/3] smaller buffer --- .../tests/FileSystemWatcher.InternalBufferSize.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.InternalBufferSize.cs b/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.InternalBufferSize.cs index bf5441a4fd189..91fe67ed6c8d0 100644 --- a/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.InternalBufferSize.cs +++ b/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.InternalBufferSize.cs @@ -39,7 +39,7 @@ public class InternalBufferSizeTests : FileSystemWatcherTest // At some point we might decide to improve how FSW handles this at which point we'll need // a better test for Error (perhaps just a mock), but for now there is some value in forcing this limit. - private const int ExcessEventsMultiplier = 250; // 100 does not reliably trigger the error + private const int ExcessEventsMultiplier = 200; // 100 does not reliably trigger the error [Theory] [InlineData(true)] @@ -52,6 +52,7 @@ public void FileSystemWatcher_InternalBufferSize(bool setToHigherCapacity) string file = CreateTestFile(TestDirectory, "file"); using (FileSystemWatcher watcher = CreateWatcher(TestDirectory, file, unblockHandler)) { + watcher.InternalBufferSize = 4096; // Minimum int internalBufferOperationCapacity = watcher.InternalBufferSize / (12 + 2 * (Path.GetFileName(file).Length + 1)); if (setToHigherCapacity) @@ -82,6 +83,7 @@ public void FileSystemWatcher_InternalBufferSize_SynchronizingObject() TestISynchronizeInvoke invoker = new TestISynchronizeInvoke(); watcher.SynchronizingObject = invoker; + watcher.InternalBufferSize = 4096; // Minimum int internalBufferOperationCapacity = watcher.InternalBufferSize / (12 + 2 * (Path.GetFileName(file).Length + 1)); Action action = GetAction(unblockHandler, internalBufferOperationCapacity, file); From a42b063686e9e4c263e6a773b5836a594a62343b Mon Sep 17 00:00:00 2001 From: Dan Moseley Date: Sun, 23 Jun 2024 14:58:53 -0600 Subject: [PATCH 3/3] typo --- .../tests/FileSystemWatcher.InternalBufferSize.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.InternalBufferSize.cs b/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.InternalBufferSize.cs index 91fe67ed6c8d0..6df52a9395793 100644 --- a/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.InternalBufferSize.cs +++ b/src/libraries/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.InternalBufferSize.cs @@ -110,7 +110,7 @@ private Action GetAction(ManualResetEvent unblockHandler, int internalBufferOper { return () => { - List tasks = new (); + List tasks = new(); // Generate enough file change events to overflow the default buffer // For speed, do this on multiple threads