diff options
author | Aleksey Kliger (λgeek) <alklig@microsoft.com> | 2019-09-20 18:21:30 +0300 |
---|---|---|
committer | Steve Pfister <steveisok@users.noreply.github.com> | 2019-09-20 18:21:30 +0300 |
commit | 3757a7296c9cd700f9eceb8a9460a2de44e7cc01 (patch) | |
tree | b01cfb34aebebe5e0d2e58a25182417a56e34d9d /mcs | |
parent | 95d17b16a86e23cb98892c6279679800106917ec (diff) |
[System] Make FileSystemWatcher backend non-static (#16922)
* [System] Make FileSystemWatcher backend non-static
The FileSystemWatcher `watcher` field is the backend IFileWatcher that's
supposed to be used for the actual operations.
Each backend has a `bool GetInstance (out watcher)` method that returns a
singleton object, so each new instance of FileSystemWatcher gets the same
IFileWatcher instance. (They will get different `watcher_handle` instances
which are used by some backends to uniquely identify this FileSystemWatcher).
However when the first FileSystemWatcher instance is `Dispose`d, it will set
`watcher = null` which will prevent the remaining instances from calling
watcher?.StopDispatching (watcher_handle);
watcher?.Dispose (watcher_handle);
which means that the backend won't properly cleanup resources for any other FSW
instances that have other `watcher_handle` values.
Addresses https://github.com/mono/mono/issues/16709
Diffstat (limited to 'mcs')
-rw-r--r-- | mcs/class/System/System.IO/FileSystemWatcher.cs | 2 | ||||
-rw-r--r-- | mcs/class/System/Test/System.IO/FileSystemWatcherTest.cs | 78 |
2 files changed, 78 insertions, 2 deletions
diff --git a/mcs/class/System/System.IO/FileSystemWatcher.cs b/mcs/class/System/System.IO/FileSystemWatcher.cs index 9c003a80e5a..cbf71e7a9a6 100644 --- a/mcs/class/System/System.IO/FileSystemWatcher.cs +++ b/mcs/class/System/System.IO/FileSystemWatcher.cs @@ -61,7 +61,7 @@ namespace System.IO { SearchPattern2 pattern; bool disposed; string mangledFilter; - static IFileWatcher watcher; + IFileWatcher watcher; object watcher_handle; static object lockobj = new object (); diff --git a/mcs/class/System/Test/System.IO/FileSystemWatcherTest.cs b/mcs/class/System/Test/System.IO/FileSystemWatcherTest.cs index 358a43eb1cd..5e1a2b7303a 100644 --- a/mcs/class/System/Test/System.IO/FileSystemWatcherTest.cs +++ b/mcs/class/System/Test/System.IO/FileSystemWatcherTest.cs @@ -11,6 +11,7 @@ using NUnit.Framework; using System; using System.IO; +using System.Reflection; using MonoTests.Helpers; @@ -109,7 +110,82 @@ namespace MonoTests.System.IO } } } + + [Test] + public void CreateTwoAndDispose () + { + // Create two FSW instances and dispose them. Verify + // that the backend IFileWatcher's Dispose + // (watcher_handle) method got called. + + // FIXME: This only works for the + // CoreFXFileSystemWatcherProxy not the other backends. + + using (var tmp = new TempDirectory ()) { + // have to use reflection to poke at the private fields of FileSystemWatcher. + var watcherField = typeof (FileSystemWatcher).GetField ("watcher", BindingFlags.Instance | BindingFlags.NonPublic); + Assert.IsNotNull (watcherField); + var watcherHandleField = typeof (FileSystemWatcher).GetField ("watcher_handle", BindingFlags.Instance | BindingFlags.NonPublic); + Assert.IsNotNull (watcherHandleField); + var proxyType = typeof (FileSystemWatcher).Assembly.GetType ("System.IO.CoreFXFileSystemWatcherProxy"); + Assert.IsNotNull (proxyType); + // the "internal_map" maps watcher handles to backend CoreFX FSW instances + var proxyTypeInternalMapField = proxyType.GetField ("internal_map", BindingFlags.Static | BindingFlags.NonPublic); + Assert.IsNotNull (proxyTypeInternalMapField); + + var fsw1 = new FileSystemWatcher (tmp.Path, "*"); + var fsw2 = new FileSystemWatcher (tmp.Path, "*"); + // at this point watcher and watcher_handle should be set + + global::System.Collections.Generic.IDictionary<object, global::System.IO.CoreFX.FileSystemWatcher> internal_map = null; + object handle1 = null; + object handle2 = null; + + // using "using" to ensure that Dispose gets called even if we throw an exception + using (var fsw11 = fsw1) + using (var fsw22 = fsw2) { + + // Once at least one FSW is initialized, watcher should be set. But if the + // wrong backend is getting used, ignore this test because the other checks + // (internal_map in particular) won't be valid. + var watcher = watcherField.GetValue (fsw1); + Assert.IsNotNull (watcher); + if (!proxyType.IsAssignableFrom (watcher.GetType ())) + Assert.Ignore ("Testing only CoreFXFileSystemWatcherProxy FSW backend"); + + handle1 = watcherHandleField.GetValue (fsw1); + handle2 = watcherHandleField.GetValue (fsw2); + + Assert.IsNotNull (handle1); + Assert.IsNotNull (handle2); + + // Can't check for internal_map earlier - it is lazily created when the first + // FSW instance is created + internal_map = proxyTypeInternalMapField.GetValue (null) + as global::System.Collections.Generic.IDictionary<object, global::System.IO.CoreFX.FileSystemWatcher>; + Assert.IsNotNull (internal_map); + + // Both of handles should be in the internal map while the file system watchers + // are not disposed. + Assert.IsTrue (internal_map.ContainsKey (handle1)); + Assert.IsTrue (internal_map.ContainsKey (handle2)); + + } + + // Dispose was called, now watcher_handle should be null + + Assert.IsNull (watcherHandleField.GetValue (fsw1)); + Assert.IsNull (watcherHandleField.GetValue (fsw2)); + + // This pair are the critical checks: after we call Dispose on fsw1 and fsw2, the + // backend's internal map shouldn't have anything keyed on handle1 and handle2. + // Therefore System.IO.CoreFX.FileSystemWatcher instances will be disposed of, too. + Assert.IsFalse (internal_map.ContainsKey (handle1)); + Assert.IsFalse (internal_map.ContainsKey (handle2)); + } + } + } } -#endif
\ No newline at end of file +#endif |