Merge pull request #2674 from stephentoub/fix_fsw_threadpoolbinding

Fix FSW freeing of NativeOverlapped on wrong ThreadPoolBoundHandle

Author: stephentoub

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

@@ -108,12 +108,11 @@ private void FinalizeDispose()
         // Thread gate holder and constats
         private bool _stopListening = false;
 
-        private bool IsHandleInvalid
+        private bool IsHandleInvalid { get { return IsHandleInstanceInvalid(_directoryHandle); } }
+
+        private bool IsHandleInstanceInvalid(SafeFileHandle handle)
         {
-            get
-            {
-                return (_directoryHandle == null || _directoryHandle.IsInvalid || _directoryHandle.IsClosed);
-            }
+            return handle == null || handle.IsInvalid || handle.IsClosed;
         }
 
         /// <devdoc>
@@ -132,12 +131,21 @@ private unsafe void Monitor(byte[] buffer)
                 buffer = AllocateBuffer();
             }
 
+            // Use a local copy of _threadPoolBinding for the remainder of the method.
+            // Otherwise, the value of the field could get replaced by a different
+            // instance if events are stopped and then restarted, and we could end up
+            // trying to use the new binding with an overlapped created on the old binding.
+            // Similarly for the directory handle.
+            ThreadPoolBoundHandle threadPoolBinding = _threadPoolBinding;
+            SafeFileHandle directoryHandle = _directoryHandle;
+
             // Pass "session" counter to callback:
             FSWAsyncResult asyncResult = new FSWAsyncResult();
             asyncResult.session = _currentSession;
             asyncResult.buffer = buffer;
+            asyncResult.threadPoolBinding = threadPoolBinding;
 
-            NativeOverlapped* overlappedPointer = _threadPoolBinding.AllocateNativeOverlapped(new IOCompletionCallback(this.CompletionStatusChanged), asyncResult, buffer);
+            NativeOverlapped* overlappedPointer = threadPoolBinding.AllocateNativeOverlapped(new IOCompletionCallback(this.CompletionStatusChanged), asyncResult, buffer);
 
             // Can now call OS:
             int size;
@@ -149,12 +157,12 @@ private unsafe void Monitor(byte[] buffer)
                 // and when we get here from CompletionStatusChanged.
                 // We might need to take a lock to prevent race absolutely, instead just catch
                 // ObjectDisposedException from SafeHandle in case it is disposed
-                if (!IsHandleInvalid)
+                if (!IsHandleInstanceInvalid(directoryHandle))
                 {
                     // An interrupt is possible here
                     fixed (byte* buffPtr = buffer)
                     {
-                        ok = UnsafeNativeMethods.ReadDirectoryChangesW(_directoryHandle,
+                        ok = UnsafeNativeMethods.ReadDirectoryChangesW(directoryHandle,
                                                            buffPtr,
                                                            _internalBufferSize,
                                                            _includeSubdirectories ? 1 : 0,
@@ -170,17 +178,17 @@ private unsafe void Monitor(byte[] buffer)
             }
             catch (ArgumentNullException)
             { //Ignore
-                Debug.Assert(IsHandleInvalid, "ArgumentNullException from something other than SafeHandle?");
+                Debug.Assert(IsHandleInstanceInvalid(directoryHandle), "ArgumentNullException from something other than SafeHandle?");
             }
             finally
             {
                 if (!ok)
                 {
-                    _threadPoolBinding.FreeNativeOverlapped(overlappedPointer);
+                    threadPoolBinding.FreeNativeOverlapped(overlappedPointer);
 
                     // If the handle was for some reason changed or closed during this call, then don't throw an
                     // exception.  Else, it's a valid error.
-                    if (!IsHandleInvalid)
+                    if (!IsHandleInstanceInvalid(directoryHandle))
                     {
                         OnError(new ErrorEventArgs(new Win32Exception()));
                     }
@@ -354,7 +362,7 @@ rename event with the Name field null and then fire the next action.
             }
             finally
             {
-                _threadPoolBinding.FreeNativeOverlapped(overlappedPointer);
+                asyncResult.threadPoolBinding.FreeNativeOverlapped(overlappedPointer);
                 if (!_stopListening)
                 {
                     Monitor(asyncResult.buffer);
@@ -368,6 +376,7 @@ private sealed class FSWAsyncResult
         {
             internal int session;
             internal byte[] buffer;
+            internal ThreadPoolBoundHandle threadPoolBinding;
         }
 
         /// <devdoc>