Fix regression in FileSystemWatcher

As part of the bug fix in f2720ab , a regression was introduced, one that we didn't catch until recently due to not having a test that stressed stopping and quickly restarting an FSW doing monitoring.

At the time of that fix, the active asynchronous monitoring operation didn't have its own copy of the directory handle; instead, it just used the single field on the FSW instance.  As such, in a race condition between the asynchronous monitoring operation trying to use that instance and the main thread stopping the FSW and nulling out the field, the asynchronous monitoring operation could experience a NullReferenceException.  The fix was to stop nulling out the field and just rely on closing the handle being enough to signal that monitoring should stop.

However, this causes a problem in a particular race condition.  While a SafeHandle is in use by a P/Invoke operation, its reference count is temporarily incremented so as to prevent it from being closed while in use.  This means that if the directory handle is disposed of while it's being used by ReadDirectoryChangesW, the handle's IsClosed won't return true until after ReadDirectoryChangesW returns.  When the FSW is stopped, Dispose is called on the handle, and then when it's restarted, the handle's IsClosed is checked to see if there's currently an active monitoring operation, and if there is, the FSW assumes it doesn't need to restart anything.  But if the stop and the start both occur during the call to ReadDirectoryChangesW, then the start won't actually start anything.

My reliability/performance fixes in b6c478c eliminated the reliance on the shared directory handle field: the asynchronous monitoring operation has its own copy.  Thus, there's no longer a problem in nulling out the directory handle field.  The fix for the regression is simply to do so.

(Long explanation for a fix that's simply to null out a field :-)

Author: stephentoub

src/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Win32.cs

@@ -75,8 +75,13 @@ private void StopRaisingEvents()
             // This operation doesn't need to be atomic because the API will deal with a closed
             // handle appropriately. If we get here while asynchronously waiting on a change notification, 
             // closing the directory handle should cause ReadDirectoryChangesCallback be be called,
-            // cleaning up the operation.
+            // cleaning up the operation.  Note that it's critical to also null out the handle.  If the
+            // handle is currently in use in a P/Invoke, it will have its reference count temporarily
+            // increased, such that the disposal operation won't take effect and close the handle
+            // until that P/Invoke returns; if during that time the FSW is restarted, the IsHandleInvalid
+            // check will see a valid handle, unless we also null it out.
             _directoryHandle.Dispose();
+            _directoryHandle = null;
 
             // Set enabled to false
             _enabled = false;

src/System.IO.FileSystem.Watcher/tests/FileSystemWatcher.Changed.cs

@@ -146,7 +146,6 @@ public static void FileSystemWatcher_Changed_FileDataChange()
         }
     }
 
-    [ActiveIssue(2740)]
     [Theory]
     [InlineData(true)]
     [InlineData(false)]