[2019-08] [System] Make FileSystemWatcher backend non-static (#16926)

[2019-08] [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 #16709



<!--
Thank you for your Pull Request!

If you are new to contributing to Mono, please try to do your best at conforming to our coding guidelines http://www.mono-project.com/community/contributing/coding-guidelines/ but don't worry if you get something wrong. One of the project members will help you to get things landed.

Does your pull request fix any of the existing issues? Please use the following format: Fixes #issue-number
-->


Backport of #16922.

/cc @steveisok @lambdageek

mcs/class/System/System.IO/FileSystemWatcher.cs

@@ -61,7 +61,7 @@ public class FileSystemWatcher : Component, ISupportInitialize {
 		SearchPattern2 pattern;
 		bool disposed;
 		string mangledFilter;
-		static IFileWatcher watcher;
+		IFileWatcher watcher;
 		object watcher_handle;
 		static object lockobj = new object ();
 

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,72 @@ public void LargeNumberOfInstances ()
 				}
 			}
 		}
+
+		[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);
+
+				var fsw1 = new FileSystemWatcher (tmp.Path, "*");
+				var fsw2 = new FileSystemWatcher (tmp.Path, "*");
+
+				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) {
+					// at this point watcher and watcher_handle should be set
+
+					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);
+
+				}
+
+				// Dispose was called, now watcher_handle should be null
+
+				Assert.IsNull (watcherHandleField.GetValue (fsw1));
+				Assert.IsNull (watcherHandleField.GetValue (fsw2));
+
+				// and moreover, the CoreFXFileSystemWatcherProxy shouldn't have entries for either handle1 or handle2
+
+				var proxyTypeInternalMapField = proxyType.GetField ("internal_map", BindingFlags.Static | BindingFlags.NonPublic);
+				Assert.IsNotNull (proxyTypeInternalMapField);
+				var internal_map = proxyTypeInternalMapField.GetValue (null)
+					as global::System.Collections.Generic.IDictionary<object, global::System.IO.CoreFX.FileSystemWatcher>;
+				Assert.IsNotNull (internal_map);
+
+				// 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
+#endif