From 21799512c3d5e6a1679e9b84b96f47da54a16c3e Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Wed, 15 Feb 2023 22:28:14 -0800 Subject: [PATCH 1/2] [7.0] Check for pending IO in the portable thread pool's worker threads - Port of https://github.com/dotnet/runtime/pull/82245 - 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 Port of fix for 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 b1e9ed9a6be6ad..21ddf78f34ca5f 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.Threading.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.Threading.cs @@ -73,5 +73,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 344fdcc2af6694..9901a5de08d5e2 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 @@ -1763,6 +1763,9 @@ Common\Interop\Windows\Kernel32\Interop.SystemTimeToFileTime.cs + + Common\Interop\Windows\Kernel32\Interop.Threading.cs + Common\Interop\Windows\Kernel32\Interop.TimeZone.cs @@ -2417,9 +2420,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 18777f4a555ab1..7ec209ecc0dc1f 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 17396afc320a5e4240f7a4b4594e1b714b92bc85 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 9901a5de08d5e2..4cac14ebc579c8 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 @@ -2383,8 +2383,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 7ec209ecc0dc1f..9527f84a876604 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; }