Handle success case for overlapped IO in Windows Diagnostics IPC PAL (#39807)

Prevents unintialized memory from making its way into the call to WaitForMultipleObjects.  That memory could contain a valid handle value and cause WFMO to hang since it isn't waiting on the correct handles.

diff --git a/src/coreclr/src/debug/debug-pal/win/diagnosticsipc.cpp b/src/coreclr/src/debug/debug-pal/win/diagnosticsipc.cpp
index 6c1b55e..2b6c384 100644 (file)
--- a/src/coreclr/src/debug/debug-pal/win/diagnosticsipc.cpp
+++ b/src/coreclr/src/debug/debug-pal/win/diagnosticsipc.cpp
@@ -119,6 +119,7 @@ bool IpcStream::DiagnosticsIpc::Listen(ErrorCallback callback)
                 _hPipe = INVALID_HANDLE_VALUE;
                 ::CloseHandle(_oOverlap.hEvent);
                 _oOverlap.hEvent = INVALID_HANDLE_VALUE;
+                memset(&_oOverlap, 0, sizeof(OVERLAPPED)); // clear the overlapped objects state
                 return false;
         }
     }
@@ -193,11 +194,15 @@ IpcStream *IpcStream::DiagnosticsIpc::Connect(ErrorCallback callback)
     return new IpcStream(hPipe, mode);
 }
 
-void IpcStream::DiagnosticsIpc::Close(bool isShutdown, ErrorCallback)
+void IpcStream::DiagnosticsIpc::Close(bool isShutdown, ErrorCallback callback)
 {
     // don't attempt cleanup on shutdown and let the OS handle it
     if (isShutdown)
+    {
+        if (callback != nullptr)
+            callback("Closing without cleaning underlying handles", 100);
         return;
+    }
 
     if (_hPipe != INVALID_HANDLE_VALUE)
     {
@@ -205,15 +210,22 @@ void IpcStream::DiagnosticsIpc::Close(bool isShutdown, ErrorCallback)
         {
             const BOOL fSuccessDisconnectNamedPipe = ::DisconnectNamedPipe(_hPipe);
             _ASSERTE(fSuccessDisconnectNamedPipe != 0);
+            if (fSuccessDisconnectNamedPipe != 0 && callback != nullptr)
+                callback("Failed to disconnect NamedPipe", ::GetLastError());
         }
 
         const BOOL fSuccessCloseHandle = ::CloseHandle(_hPipe);
         _ASSERTE(fSuccessCloseHandle != 0);
+        if (fSuccessCloseHandle != 0 && callback != nullptr)
+            callback("Failed to close pipe handle", ::GetLastError());
     }
 
     if (_oOverlap.hEvent != INVALID_HANDLE_VALUE)
     {
-        ::CloseHandle(_oOverlap.hEvent);
+        const BOOL fSuccessCloseEvent = ::CloseHandle(_oOverlap.hEvent);
+        _ASSERTE(fSuccessCloseEvent != 0);
+        if (fSuccessCloseEvent != 0 && callback != nullptr)
+            callback("Failed to close overlap event handle", ::GetLastError());
     }
 }
 
@@ -230,7 +242,7 @@ IpcStream::~IpcStream()
     Close();
 }
 
-void IpcStream::Close(ErrorCallback)
+void IpcStream::Close(ErrorCallback callback)
 {
     if (_hPipe != INVALID_HANDLE_VALUE)
     {
@@ -240,15 +252,22 @@ void IpcStream::Close(ErrorCallback)
         {
             const BOOL fSuccessDisconnectNamedPipe = ::DisconnectNamedPipe(_hPipe);
             _ASSERTE(fSuccessDisconnectNamedPipe != 0);
+            if (fSuccessDisconnectNamedPipe != 0 && callback != nullptr)
+                callback("Failed to disconnect NamedPipe", ::GetLastError());
         }
 
         const BOOL fSuccessCloseHandle = ::CloseHandle(_hPipe);
         _ASSERTE(fSuccessCloseHandle != 0);
+        if (fSuccessCloseHandle != 0 && callback != nullptr)
+            callback("Failed to close pipe handle", ::GetLastError());
     }
 
     if (_oOverlap.hEvent != INVALID_HANDLE_VALUE)
     {
-        ::CloseHandle(_oOverlap.hEvent);
+        const BOOL fSuccessCloseEvent = ::CloseHandle(_oOverlap.hEvent);
+        _ASSERTE(fSuccessCloseEvent != 0);
+        if (fSuccessCloseEvent != 0 && callback != nullptr)
+            callback("Failed to close overlapped event handle", ::GetLastError());
     }
 }
 
@@ -302,6 +321,11 @@ int32_t IpcStream::DiagnosticsIpc::Poll(IpcPollHandle *rgIpcPollHandles, uint32_
                             return -1;
                     }
                 }
+                else
+                {
+                    // there's already data to be read
+                    pHandles[i] = rgIpcPollHandles[i].pStream->_oOverlap.hEvent;
+                }
             }
             else
             {

diff --git a/src/coreclr/tests/issues.targets b/src/coreclr/tests/issues.targets
index 4cb3d04..f15a2b5 100644 (file)
--- a/src/coreclr/tests/issues.targets
+++ b/src/coreclr/tests/issues.targets
@@ -306,9 +306,6 @@
 
     <!-- Windows all architecture excludes -->
     <ItemGroup Condition="'$(XunitTestBinBase)' != '' and '$(TargetsWindows)' == 'true' and '$(RuntimeFlavor)' == 'coreclr'">
-        <ExcludeList Include="$(XunitTestBinBase)/tracing/eventpipe/pauseonstart/pauseonstart/**">
-            <Issue>https://github.com/dotnet/runtime/issues/38847</Issue>
-        </ExcludeList>
     </ItemGroup>
 
     <!-- Windows x64 specific excludes -->