From e4d78fc5e4792a76e9f96683c3efd4c225610e92 Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Wed, 15 Feb 2023 22:28:14 -0800 Subject: [PATCH 1/2] Check for pending IO in the portable thread pool's worker threads - When Resource Monitor is attached, some async IO operations are bound to the thread that issued it even though the IO handle is bound to an IOCP. If the thread exits, the async IO operation is aborted. This can lead to hangs or unexpected exceptions. - Added a check that was missing in the portable thread pool implementation to prevent exiting a worker thread when it has pending IO Fixes https://github.com/dotnet/runtime/issues/82207 --- .../Interop/Windows/Kernel32/Interop.Threading.cs | 4 ++++ .../src/System.Private.CoreLib.Shared.projitems | 6 +++--- .../Threading/PortableThreadPool.WorkerThread.cs | 12 ++++++++++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.Threading.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.Threading.cs index f64a23f7722344..9a53b48bec17ce 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.Threading.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.Threading.cs @@ -70,5 +70,9 @@ internal enum ThreadPriority : int [LibraryImport(Libraries.Kernel32)] [return:MarshalAs(UnmanagedType.Bool)] internal static partial bool SetThreadPriority(SafeWaitHandle hThread, int nPriority); + + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool GetThreadIOPendingFlag(nint hThread, out BOOL lpIOIsPending); } } diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 24b9d04497aa84..05575e87d94e8e 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -1832,6 +1832,9 @@ Common\Interop\Windows\Kernel32\Interop.SystemTimeToFileTime.cs + + Common\Interop\Windows\Kernel32\Interop.Threading.cs + Common\Interop\Windows\Kernel32\Interop.TimeZone.cs @@ -2512,9 +2515,6 @@ - - Interop\Windows\Kernel32\Interop.Threading.cs - diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.cs index 0e0323e3bc446c..354d691758f04d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics; using System.Diagnostics.Tracing; namespace System.Threading @@ -115,6 +116,17 @@ private static void WorkerThreadStart() } } + // The thread cannot exit if it has IO pending, otherwise the IO may be canceled + bool success = + Interop.Kernel32.GetThreadIOPendingFlag( + Interop.Kernel32.GetCurrentThread(), + out Interop.BOOL isIOPending); + Debug.Assert(success); + if (!success || isIOPending != Interop.BOOL.FALSE) + { + continue; + } + threadAdjustmentLock.Acquire(); try { From 0fedfbb54792139eb607864d9cad4e24ee3cf28c Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Thu, 16 Feb 2023 10:11:47 -0800 Subject: [PATCH 2/2] Refactor Windows-specific code --- .../src/System.Private.CoreLib.Shared.projitems | 4 ++-- ...onReader.Unix.cs => PortableThreadPool.Unix.cs} | 5 +++++ ...er.Windows.cs => PortableThreadPool.Windows.cs} | 14 ++++++++++++++ .../Threading/PortableThreadPool.WorkerThread.cs | 10 ++-------- 4 files changed, 23 insertions(+), 10 deletions(-) rename src/libraries/System.Private.CoreLib/src/System/Threading/{PortableThreadPool.CpuUtilizationReader.Unix.cs => PortableThreadPool.Unix.cs} (80%) rename src/libraries/System.Private.CoreLib/src/System/Threading/{PortableThreadPool.CpuUtilizationReader.Windows.cs => PortableThreadPool.Windows.cs} (75%) diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 05575e87d94e8e..2dac79c9d7d4c5 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -2480,8 +2480,8 @@ - - + + diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.CpuUtilizationReader.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.Unix.cs similarity index 80% rename from src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.CpuUtilizationReader.Unix.cs rename to src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.Unix.cs index ea754f9185e9bf..100ddf0c675d13 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.CpuUtilizationReader.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.Unix.cs @@ -5,6 +5,11 @@ namespace System.Threading { internal sealed partial class PortableThreadPool { + private static partial class WorkerThread + { + private static bool IsIOPending => false; + } + private struct CpuUtilizationReader { private Interop.Sys.ProcessCpuInformation _cpuInfo; diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.CpuUtilizationReader.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.Windows.cs similarity index 75% rename from src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.CpuUtilizationReader.Windows.cs rename to src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.Windows.cs index 4818512ba8183a..6f5d7eff9d96c9 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.CpuUtilizationReader.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.Windows.cs @@ -8,6 +8,20 @@ namespace System.Threading { internal sealed partial class PortableThreadPool { + private static partial class WorkerThread + { + private static bool IsIOPending + { + get + { + bool success = + Interop.Kernel32.GetThreadIOPendingFlag(Interop.Kernel32.GetCurrentThread(), out Interop.BOOL isIOPending); + Debug.Assert(success); + return !success || isIOPending != Interop.BOOL.FALSE; + } + } + } + private struct CpuUtilizationReader { public long _idleTime; diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.cs index 354d691758f04d..96578b9de6b8dd 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Diagnostics; using System.Diagnostics.Tracing; namespace System.Threading @@ -11,7 +10,7 @@ internal sealed partial class PortableThreadPool /// /// The worker thread infastructure for the CLR thread pool. /// - private static class WorkerThread + private static partial class WorkerThread { private const int SemaphoreSpinCountDefaultBaseline = 70; #if !TARGET_ARM64 && !TARGET_ARM && !TARGET_LOONGARCH64 @@ -117,12 +116,7 @@ private static void WorkerThreadStart() } // The thread cannot exit if it has IO pending, otherwise the IO may be canceled - bool success = - Interop.Kernel32.GetThreadIOPendingFlag( - Interop.Kernel32.GetCurrentThread(), - out Interop.BOOL isIOPending); - Debug.Assert(success); - if (!success || isIOPending != Interop.BOOL.FALSE) + if (IsIOPending) { continue; }