From 457ed114aebe9798fd58bbf9733194e51b734896 Mon Sep 17 00:00:00 2001 From: Eric StJohn Date: Thu, 12 Nov 2020 07:16:18 -0800 Subject: [PATCH] Make PhysicalFilesWatcher exclusively use polling when pollForChanges=true and no FSW is provided (#41426) When a FileSystemWatcher is not passed to the PhysicalFileWatcher we'll permit construction (if polling is enabled) and have it behave as exclusively polling. The PhysicalFileProvider will use this mode when both UsePollingFileWatcher and UseActivePolling are set which is what happens when the DOTNET_USE_POLLING_FILE_WATCHER environment variable is set. --- .../src/PhysicalFileProvider.cs | 5 +- .../src/PhysicalFilesWatcher.cs | 51 ++++++--- .../src/Resources/Strings.resx | 123 +++++++++++++++++++++ .../tests/PhysicalFileProviderTests.cs | 76 +++++++++++++ 4 files changed, 236 insertions(+), 19 deletions(-) create mode 100644 src/libraries/Microsoft.Extensions.FileProviders.Physical/src/Resources/Strings.resx diff --git a/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PhysicalFileProvider.cs b/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PhysicalFileProvider.cs index ea06ffb..9ac3085 100644 --- a/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PhysicalFileProvider.cs +++ b/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PhysicalFileProvider.cs @@ -159,7 +159,10 @@ namespace Microsoft.Extensions.FileProviders internal PhysicalFilesWatcher CreateFileWatcher() { string root = PathUtils.EnsureTrailingSlash(Path.GetFullPath(Root)); - return new PhysicalFilesWatcher(root, new FileSystemWatcher(root), UsePollingFileWatcher, _filters) + + // When both UsePollingFileWatcher & UseActivePolling are set, we won't use a FileSystemWatcher. + FileSystemWatcher watcher = UsePollingFileWatcher && UseActivePolling ? null : new FileSystemWatcher(root); + return new PhysicalFilesWatcher(root, watcher, UsePollingFileWatcher, _filters) { UseActivePolling = UseActivePolling, }; diff --git a/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PhysicalFilesWatcher.cs b/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PhysicalFilesWatcher.cs index b819c83..f9f1223 100644 --- a/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PhysicalFilesWatcher.cs +++ b/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/PhysicalFilesWatcher.cs @@ -79,14 +79,23 @@ namespace Microsoft.Extensions.FileProviders.Physical bool pollForChanges, ExclusionFilters filters) { + if (fileSystemWatcher == null && !pollForChanges) + { + throw new ArgumentNullException(nameof(fileSystemWatcher), SR.Error_FileSystemWatcherRequiredWithoutPolling); + } + _root = root; - _fileWatcher = fileSystemWatcher; - _fileWatcher.IncludeSubdirectories = true; - _fileWatcher.Created += OnChanged; - _fileWatcher.Changed += OnChanged; - _fileWatcher.Renamed += OnRenamed; - _fileWatcher.Deleted += OnChanged; - _fileWatcher.Error += OnError; + + if (fileSystemWatcher != null) + { + _fileWatcher = fileSystemWatcher; + _fileWatcher.IncludeSubdirectories = true; + _fileWatcher.Created += OnChanged; + _fileWatcher.Changed += OnChanged; + _fileWatcher.Renamed += OnRenamed; + _fileWatcher.Deleted += OnChanged; + _fileWatcher.Error += OnError; + } PollForChanges = pollForChanges; _filters = filters; @@ -361,27 +370,33 @@ namespace Microsoft.Extensions.FileProviders.Physical private void TryDisableFileSystemWatcher() { - lock (_fileWatcherLock) + if (_fileWatcher != null) { - if (_filePathTokenLookup.IsEmpty && - _wildcardTokenLookup.IsEmpty && - _fileWatcher.EnableRaisingEvents) + lock (_fileWatcherLock) { - // Perf: Turn off the file monitoring if no files to monitor. - _fileWatcher.EnableRaisingEvents = false; + if (_filePathTokenLookup.IsEmpty && + _wildcardTokenLookup.IsEmpty && + _fileWatcher.EnableRaisingEvents) + { + // Perf: Turn off the file monitoring if no files to monitor. + _fileWatcher.EnableRaisingEvents = false; + } } } } private void TryEnableFileSystemWatcher() { - lock (_fileWatcherLock) + if (_fileWatcher != null) { - if ((!_filePathTokenLookup.IsEmpty || !_wildcardTokenLookup.IsEmpty) && - !_fileWatcher.EnableRaisingEvents) + lock (_fileWatcherLock) { - // Perf: Turn off the file monitoring if no files to monitor. - _fileWatcher.EnableRaisingEvents = true; + if ((!_filePathTokenLookup.IsEmpty || !_wildcardTokenLookup.IsEmpty) && + !_fileWatcher.EnableRaisingEvents) + { + // Perf: Turn off the file monitoring if no files to monitor. + _fileWatcher.EnableRaisingEvents = true; + } } } } diff --git a/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/Resources/Strings.resx b/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/Resources/Strings.resx new file mode 100644 index 0000000..061407c --- /dev/null +++ b/src/libraries/Microsoft.Extensions.FileProviders.Physical/src/Resources/Strings.resx @@ -0,0 +1,123 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + The fileSystemWatcher parameter must be non-null when pollForChanges is false. + + \ No newline at end of file diff --git a/src/libraries/Microsoft.Extensions.FileProviders.Physical/tests/PhysicalFileProviderTests.cs b/src/libraries/Microsoft.Extensions.FileProviders.Physical/tests/PhysicalFileProviderTests.cs index 0d9fb95..97b6338 100644 --- a/src/libraries/Microsoft.Extensions.FileProviders.Physical/tests/PhysicalFileProviderTests.cs +++ b/src/libraries/Microsoft.Extensions.FileProviders.Physical/tests/PhysicalFileProviderTests.cs @@ -2,9 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using System.IO; using System.Linq; using System.Runtime.InteropServices; +using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.FileProviders.Internal; using Microsoft.Extensions.FileProviders.Physical; @@ -16,6 +18,7 @@ namespace Microsoft.Extensions.FileProviders public class PhysicalFileProviderTests { private const int WaitTimeForTokenToFire = 500; + private const int WaitTimeForTokenCallback = 10000; [Fact] public void GetFileInfoReturnsNotFoundFileInfoForNullPath() @@ -87,6 +90,54 @@ namespace Microsoft.Extensions.FileProviders GetFileInfoReturnsNotFoundFileInfoForIllegalPathWithLeadingSlashes(path); } + [Fact] + [PlatformSpecific(TestPlatforms.Linux)] + public void PollingFileProviderShouldntConsumeINotifyInstances() + { + List disposables = new List(); + using (var root = new DisposableFileSystem()) + { + string maxInstancesFile = "/proc/sys/fs/inotify/max_user_instances"; + Assert.True(File.Exists(maxInstancesFile)); + int maxInstances = int.Parse(File.ReadAllText(maxInstancesFile)); + + // choose an arbitrary number that exceeds max + int instances = maxInstances + 16; + + AutoResetEvent are = new AutoResetEvent(false); + + var oldPollingInterval = PhysicalFilesWatcher.DefaultPollingInterval; + try + { + PhysicalFilesWatcher.DefaultPollingInterval = TimeSpan.FromMilliseconds(WaitTimeForTokenToFire); + for (int i = 0; i < instances; i++) + { + PhysicalFileProvider pfp = new PhysicalFileProvider(root.RootPath) + { + UsePollingFileWatcher = true, + UseActivePolling = true + }; + disposables.Add(pfp); + disposables.Add(pfp.Watch("*").RegisterChangeCallback(_ => are.Set(), null)); + } + + // trigger an event + root.CreateFile("test.txt"); + + // wait for at least one event. + Assert.True(are.WaitOne(WaitTimeForTokenCallback)); + } + finally + { + PhysicalFilesWatcher.DefaultPollingInterval = oldPollingInterval; + foreach (var disposable in disposables) + { + disposable.Dispose(); + } + } + } + } + private void GetFileInfoReturnsNotFoundFileInfoForIllegalPathWithLeadingSlashes(string path) { using (var provider = new PhysicalFileProvider(Path.GetTempPath())) @@ -1479,5 +1530,30 @@ namespace Microsoft.Extensions.FileProviders Assert.True(fileWatcher.UseActivePolling); } } + + [Theory] + [InlineData(false)] + [InlineData(true)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/34580", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] + public async Task CanDeleteWatchedDirectory(bool useActivePolling) + { + using (var root = new DisposableFileSystem()) + using (var provider = new PhysicalFileProvider(root.RootPath)) + { + var fileName = Path.GetRandomFileName(); + PollingFileChangeToken.PollingInterval = TimeSpan.FromMilliseconds(10); + + provider.UsePollingFileWatcher = true; // We must use polling due to https://github.com/dotnet/runtime/issues/44484 + provider.UseActivePolling = useActivePolling; + + root.CreateFile(fileName); + var token = provider.Watch(fileName); + Directory.Delete(root.RootPath, true); + + await Task.Delay(WaitTimeForTokenToFire).ConfigureAwait(false); + + Assert.True(token.HasChanged); + } + } } } -- 2.7.4