From 1e266dae32c4107600766582085c7b100d9d52fb Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Wed, 17 Jan 2024 07:10:50 -0800 Subject: [PATCH 1/2] Add an internal mode to `Lock` to have it use non-alertable waits - Added an internal constructor that enables the lock to use non-alertable waits - Non-alertable waits are not forwarded to `SynchronizationContext` wait overrides, are non-message-pumping waits, and are not interruptible - Updated most of the uses of `Lock` in NativeAOT to use non-alertable waits - Also simplified the fix in https://github.com/dotnet/runtime/pull/94873 to avoid having to do the relevant null checks in various places along the wait path, by limiting the scope of the null checks to the initialization phase Fixes https://github.com/dotnet/runtime/issues/97105 --- .../System/Threading/WaitHandle.CoreCLR.cs | 2 +- .../Concurrent/ConcurrentUnifier.cs | 2 +- .../Concurrent/ConcurrentUnifierW.cs | 2 +- .../Concurrent/ConcurrentUnifierWKeyed.cs | 2 +- .../src/CompatibilitySuppressions.xml | 4 ++ .../Runtime/FrozenObjectHeapManager.cs | 10 +---- .../ClassConstructorRunner.cs | 6 +-- .../InteropServices/ComWrappers.NativeAot.cs | 2 +- .../src/System/Threading/Condition.cs | 1 + .../src/System/Threading/Lock.NativeAot.cs | 27 ++++++++++-- .../src/System/Threading/SyncTable.cs | 2 +- .../Threading/Thread.NativeAot.Windows.cs | 2 +- .../src/System/Threading/Thread.NativeAot.cs | 2 +- .../src/System/Type.NativeAot.cs | 9 +--- ...ronment.ConstructedGenericsRegistration.cs | 2 +- .../TypeLoaderEnvironment.StaticsLookup.cs | 2 +- .../TypeLoader/TypeLoaderEnvironment.cs | 2 +- .../TypeLoader/TypeSystemContextFactory.cs | 2 +- src/coreclr/vm/comwaithandle.cpp | 5 ++- src/coreclr/vm/comwaithandle.h | 2 +- .../src/System/Threading/Lock.cs | 42 +++++++++++++++---- .../src/System/Threading/WaitHandle.Unix.cs | 4 +- .../System/Threading/WaitHandle.Windows.cs | 11 ++--- .../src/System/Threading/WaitHandle.cs | 29 +++++++------ 24 files changed, 109 insertions(+), 65 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/WaitHandle.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/WaitHandle.CoreCLR.cs index 1427473092185b..02982474ddd076 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Threading/WaitHandle.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/WaitHandle.CoreCLR.cs @@ -9,7 +9,7 @@ namespace System.Threading public abstract partial class WaitHandle { [MethodImpl(MethodImplOptions.InternalCall)] - private static extern int WaitOneCore(IntPtr waitHandle, int millisecondsTimeout); + private static extern int WaitOneCore(IntPtr waitHandle, int millisecondsTimeout, bool alertable); private static unsafe int WaitMultipleIgnoringSyncContextCore(Span waitHandles, bool waitAll, int millisecondsTimeout) { diff --git a/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifier.cs b/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifier.cs index a790e0aeb50105..4147ec0127bdd0 100644 --- a/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifier.cs +++ b/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifier.cs @@ -65,7 +65,7 @@ internal abstract class ConcurrentUnifier { protected ConcurrentUnifier() { - _lock = new Lock(); + _lock = new Lock(useAlertableWaits: false); _container = new Container(this); } diff --git a/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierW.cs b/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierW.cs index 69cfde1e48e3b7..80792b735a3e1c 100644 --- a/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierW.cs +++ b/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierW.cs @@ -75,7 +75,7 @@ internal abstract class ConcurrentUnifierW { protected ConcurrentUnifierW() { - _lock = new Lock(); + _lock = new Lock(useAlertableWaits: false); _container = new Container(this); } diff --git a/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierWKeyed.cs b/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierWKeyed.cs index 9ce60a51518574..0426d2387cdd41 100644 --- a/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierWKeyed.cs +++ b/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierWKeyed.cs @@ -84,7 +84,7 @@ internal abstract class ConcurrentUnifierWKeyed { protected ConcurrentUnifierWKeyed() { - _lock = new Lock(); + _lock = new Lock(useAlertableWaits: false); _container = new Container(this); } diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/CompatibilitySuppressions.xml b/src/coreclr/nativeaot/System.Private.CoreLib/src/CompatibilitySuppressions.xml index 805f58f68fa955..3161e1aa4c1ced 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/CompatibilitySuppressions.xml +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/CompatibilitySuppressions.xml @@ -913,6 +913,10 @@ CP0002 M:System.Reflection.MethodBase.GetParametersAsSpan + + CP0002 + M:System.Threading.Lock.#ctor(System.Boolean) + CP0015 M:System.Diagnostics.Tracing.EventSource.Write``1(System.String,``0):[T:System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute] diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/FrozenObjectHeapManager.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/FrozenObjectHeapManager.cs index d81a5409bf42ed..5782c7d6a2452d 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/FrozenObjectHeapManager.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/FrozenObjectHeapManager.cs @@ -16,7 +16,7 @@ internal unsafe partial class FrozenObjectHeapManager { public static readonly FrozenObjectHeapManager Instance = new FrozenObjectHeapManager(); - private readonly LowLevelLock m_Crst = new LowLevelLock(); + private readonly Lock m_Crst = new Lock(useAlertableWaits: false); private FrozenObjectSegment m_CurrentSegment; // Default size to reserve for a frozen segment @@ -34,9 +34,7 @@ internal unsafe partial class FrozenObjectHeapManager { HalfBakedObject* obj = null; - m_Crst.Acquire(); - - try + using (m_Crst.EnterScope()) { Debug.Assert(type != null); // _ASSERT(FOH_COMMIT_SIZE >= MIN_OBJECT_SIZE); @@ -84,10 +82,6 @@ internal unsafe partial class FrozenObjectHeapManager Debug.Assert(obj != null); } } // end of m_Crst lock - finally - { - m_Crst.Release(); - } IntPtr result = (IntPtr)obj; diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/ClassConstructorRunner.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/ClassConstructorRunner.cs index 737a3db6c59842..5f7bfd49cf9730 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/ClassConstructorRunner.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/ClassConstructorRunner.cs @@ -275,7 +275,7 @@ public static CctorHandle GetCctor(StaticClassConstructionContext* pContext) #if TARGET_WASM if (s_cctorGlobalLock == null) { - Interlocked.CompareExchange(ref s_cctorGlobalLock, new Lock(), null); + Interlocked.CompareExchange(ref s_cctorGlobalLock, new Lock(useAlertableWaits: false), null); } if (s_cctorArrays == null) { @@ -342,7 +342,7 @@ public static CctorHandle GetCctor(StaticClassConstructionContext* pContext) Debug.Assert(resultArray[resultIndex]._pContext == default(StaticClassConstructionContext*)); resultArray[resultIndex]._pContext = pContext; - resultArray[resultIndex].Lock = new Lock(); + resultArray[resultIndex].Lock = new Lock(useAlertableWaits: false); s_count++; } @@ -489,7 +489,7 @@ public static CctorHandle GetCctorThatThreadIsBlockedOn(int managedThreadId) internal static void Initialize() { s_cctorArrays = new Cctor[10][]; - s_cctorGlobalLock = new Lock(); + s_cctorGlobalLock = new Lock(useAlertableWaits: false); } [Conditional("ENABLE_NOISY_CCTOR_LOG")] diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs index 4d404018b0a079..6e25448024fc08 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs @@ -42,7 +42,7 @@ public abstract partial class ComWrappers private static readonly List s_referenceTrackerNativeObjectWrapperCache = new List(); private readonly ConditionalWeakTable _ccwTable = new ConditionalWeakTable(); - private readonly Lock _lock = new Lock(); + private readonly Lock _lock = new Lock(useAlertableWaits: false); private readonly Dictionary _rcwCache = new Dictionary(); internal static bool TryGetComInstanceForIID(object obj, Guid iid, out IntPtr unknown, out long wrapperId) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Condition.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Condition.cs index 70843a3b940fcc..c4ba1c7a1ba927 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Condition.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Condition.cs @@ -114,6 +114,7 @@ public unsafe bool Wait(int millisecondsTimeout, object? associatedObjectForMoni success = waiter.ev.WaitOneNoCheck( millisecondsTimeout, + true, // alertable associatedObjectForMonitorWait, associatedObjectForMonitorWait != null ? NativeRuntimeEventSource.WaitHandleWaitSourceMap.MonitorWait diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Lock.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Lock.NativeAot.cs index 318a8cc768024e..78fc7745401990 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Lock.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Lock.NativeAot.cs @@ -92,6 +92,18 @@ internal void Reenter(uint previousRecursionCount) _recursionCount = previousRecursionCount; } + private static bool IsFullyInitialized + { + get + { + // If NativeRuntimeEventSource is already being class-constructed by this thread earlier in the stack, Log can + // be null. This property is used to avoid going down the wait path in that case to avoid null checks in several + // other places. + Debug.Assert((StaticsInitializationStage)s_staticsInitializationStage == StaticsInitializationStage.Complete); + return NativeRuntimeEventSource.Log != null; + } + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] private TryLockResult LazyInitializeOrEnter() { @@ -101,6 +113,10 @@ private TryLockResult LazyInitializeOrEnter() case StaticsInitializationStage.Complete: if (_spinCount == SpinCountNotInitialized) { + if (!IsFullyInitialized) + { + goto case StaticsInitializationStage.Started; + } _spinCount = s_maxSpinCount; } return TryLockResult.Spin; @@ -121,7 +137,7 @@ private TryLockResult LazyInitializeOrEnter() } stage = (StaticsInitializationStage)Volatile.Read(ref s_staticsInitializationStage); - if (stage == StaticsInitializationStage.Complete) + if (stage == StaticsInitializationStage.Complete && IsFullyInitialized) { goto case StaticsInitializationStage.Complete; } @@ -166,14 +182,17 @@ private static bool TryInitializeStatics() return true; } + bool isFullyInitialized; try { s_isSingleProcessor = Environment.IsSingleProcessor; s_maxSpinCount = DetermineMaxSpinCount(); s_minSpinCount = DetermineMinSpinCount(); - // Also initialize some types that are used later to prevent potential class construction cycles - _ = NativeRuntimeEventSource.Log; + // Also initialize some types that are used later to prevent potential class construction cycles. If + // NativeRuntimeEventSource is already being class-constructed by this thread earlier in the stack, Log can be + // null. Avoid going down the wait path in that case to avoid null checks in several other places. + isFullyInitialized = NativeRuntimeEventSource.Log != null; } catch { @@ -182,7 +201,7 @@ private static bool TryInitializeStatics() } Volatile.Write(ref s_staticsInitializationStage, (int)StaticsInitializationStage.Complete); - return true; + return isFullyInitialized; } // Returns false until the static variable is lazy-initialized diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/SyncTable.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/SyncTable.cs index 02d7b4167ca6b2..0ce15862a20197 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/SyncTable.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/SyncTable.cs @@ -65,7 +65,7 @@ internal static class SyncTable /// /// Protects all mutable operations on s_entries, s_freeEntryList, s_unusedEntryIndex. Also protects growing the table. /// - internal static readonly Lock s_lock = new Lock(); + internal static readonly Lock s_lock = new Lock(useAlertableWaits: false); /// /// The dynamically growing array of sync entries. diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs index d0a48bed6c8d57..5e58f9f93b57a5 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs @@ -167,7 +167,7 @@ private bool JoinInternal(int millisecondsTimeout) } else { - result = WaitHandle.WaitOneCore(waitHandle.DangerousGetHandle(), millisecondsTimeout); + result = WaitHandle.WaitOneCore(waitHandle.DangerousGetHandle(), millisecondsTimeout, alertable: true); } return result == (int)Interop.Kernel32.WAIT_OBJECT_0; diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.cs index 5bdb703b445279..0cf8e0381b538a 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.cs @@ -31,7 +31,7 @@ public sealed partial class Thread private Exception? _startException; // Protects starting the thread and setting its priority - private Lock _lock = new Lock(); + private Lock _lock = new Lock(useAlertableWaits: false); // This is used for a quick check on thread pool threads after running a work item to determine if the name, background // state, or priority were changed by the work item, and if so to reset it. Other threads may also change some of those, diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Type.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Type.NativeAot.cs index d9c30882033000..1fc6c3535b4c0b 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Type.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Type.NativeAot.cs @@ -42,15 +42,14 @@ internal static unsafe RuntimeType GetTypeFromMethodTable(MethodTable* pMT) private static class AllocationLockHolder { - public static LowLevelLock AllocationLock = new LowLevelLock(); + public static Lock AllocationLock = new Lock(useAlertableWaits: false); } [MethodImpl(MethodImplOptions.NoInlining)] private static unsafe RuntimeType GetTypeFromMethodTableSlow(MethodTable* pMT) { // Allocate and set the RuntimeType under a lock - there's no way to free it if there is a race. - AllocationLockHolder.AllocationLock.Acquire(); - try + using (AllocationLockHolder.AllocationLock.EnterScope()) { ref RuntimeType? runtimeTypeCache = ref Unsafe.AsRef(pMT->WritableData); if (runtimeTypeCache != null) @@ -66,10 +65,6 @@ private static unsafe RuntimeType GetTypeFromMethodTableSlow(MethodTable* pMT) return type; } - finally - { - AllocationLockHolder.AllocationLock.Release(); - } } // diff --git a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.ConstructedGenericsRegistration.cs b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.ConstructedGenericsRegistration.cs index cebd5d917896fe..794ade7a16aab9 100644 --- a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.ConstructedGenericsRegistration.cs +++ b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.ConstructedGenericsRegistration.cs @@ -24,7 +24,7 @@ internal struct DynamicGenericsRegistrationData } // To keep the synchronization simple, we execute all dynamic generic type registration/lookups under a global lock - private Lock _dynamicGenericsLock = new Lock(); + private Lock _dynamicGenericsLock = new Lock(useAlertableWaits: false); internal void RegisterDynamicGenericTypesAndMethods(DynamicGenericsRegistrationData registrationData) { diff --git a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.StaticsLookup.cs b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.StaticsLookup.cs index e442bfa940ea72..8113d0da35c496 100644 --- a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.StaticsLookup.cs +++ b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.StaticsLookup.cs @@ -15,7 +15,7 @@ namespace Internal.Runtime.TypeLoader public sealed partial class TypeLoaderEnvironment { // To keep the synchronization simple, we execute all TLS registration/lookups under a global lock - private Lock _threadStaticsLock = new Lock(); + private Lock _threadStaticsLock = new Lock(useAlertableWaits: false); // Counter to keep track of generated offsets for TLS cells of dynamic types; private LowLevelDictionary _maxThreadLocalIndex = new LowLevelDictionary(); diff --git a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.cs b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.cs index 46ebec4d4d650b..eddba6a457c7af 100644 --- a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.cs +++ b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.cs @@ -145,7 +145,7 @@ internal static void Initialize() } // To keep the synchronization simple, we execute all type loading under a global lock - private Lock _typeLoaderLock = new Lock(); + private Lock _typeLoaderLock = new Lock(useAlertableWaits: false); public void VerifyTypeLoaderLockHeld() { diff --git a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeSystemContextFactory.cs b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeSystemContextFactory.cs index 7f3037fc9a0cbb..79b2cd44d2199d 100644 --- a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeSystemContextFactory.cs +++ b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeSystemContextFactory.cs @@ -18,7 +18,7 @@ public static class TypeSystemContextFactory // This allows us to avoid recreating the type resolution context again and again, but still allows it to go away once the types are no longer being built private static GCHandle s_cachedContext = GCHandle.Alloc(null, GCHandleType.Weak); - private static Lock s_lock = new Lock(); + private static Lock s_lock = new Lock(useAlertableWaits: false); public static TypeSystemContext Create() { diff --git a/src/coreclr/vm/comwaithandle.cpp b/src/coreclr/vm/comwaithandle.cpp index 8ec141aa2a4e9d..3c28bc407ff282 100644 --- a/src/coreclr/vm/comwaithandle.cpp +++ b/src/coreclr/vm/comwaithandle.cpp @@ -16,7 +16,7 @@ #include "excep.h" #include "comwaithandle.h" -FCIMPL2(INT32, WaitHandleNative::CorWaitOneNative, HANDLE handle, INT32 timeout) +FCIMPL3(INT32, WaitHandleNative::CorWaitOneNative, HANDLE handle, INT32 timeout, CLR_BOOL alertable) { FCALL_CONTRACT; @@ -28,7 +28,8 @@ FCIMPL2(INT32, WaitHandleNative::CorWaitOneNative, HANDLE handle, INT32 timeout) Thread* pThread = GET_THREAD(); - retVal = pThread->DoAppropriateWait(1, &handle, TRUE, timeout, (WaitMode)(WaitMode_Alertable | WaitMode_IgnoreSyncCtx)); + WaitMode waitMode = (WaitMode)((alertable ? WaitMode_Alertable : WaitMode_None) | WaitMode_IgnoreSyncCtx); + retVal = pThread->DoAppropriateWait(1, &handle, TRUE, timeout, waitMode); HELPER_METHOD_FRAME_END(); return retVal; diff --git a/src/coreclr/vm/comwaithandle.h b/src/coreclr/vm/comwaithandle.h index ae250a1b9a966f..ccab4ae1566305 100644 --- a/src/coreclr/vm/comwaithandle.h +++ b/src/coreclr/vm/comwaithandle.h @@ -18,7 +18,7 @@ class WaitHandleNative { public: - static FCDECL2(INT32, CorWaitOneNative, HANDLE handle, INT32 timeout); + static FCDECL3(INT32, CorWaitOneNative, HANDLE handle, INT32 timeout, CLR_BOOL alertable); static FCDECL4(INT32, CorWaitMultipleNative, HANDLE *handleArray, INT32 numHandles, CLR_BOOL waitForAll, INT32 timeout); static FCDECL3(INT32, CorSignalAndWaitOneNative, HANDLE waitHandleSignalUNSAFE, HANDLE waitHandleWaitUNSAFE, INT32 timeout); }; diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs index 056ff96d41b1ea..ade8229eae1ffb 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs @@ -38,9 +38,25 @@ public sealed partial class Lock private uint _state; // see State for layout private uint _recursionCount; private short _spinCount; - private ushort _waiterStartTimeMs; + + // The lowest bit is a flag, when set it indicates that the lock should not use alertable waits + private ushort _waiterStartTimeMsAndFlags; + private AutoResetEvent? _waitEvent; +#if NATIVEAOT // The method needs to be public in NativeAOT so that other private libraries can access it + public Lock(bool useAlertableWaits) +#else + internal Lock(bool useAlertableWaits) +#endif + : this() + { + if (!useAlertableWaits) + { + _waiterStartTimeMsAndFlags = 1; + } + } + /// /// Enters the lock. Once the method returns, the calling thread would be the only thread that holds the lock. /// @@ -444,9 +460,9 @@ private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId) Wait: bool areContentionEventsEnabled = - NativeRuntimeEventSource.Log?.IsEnabled( + NativeRuntimeEventSource.Log.IsEnabled( EventLevel.Informational, - NativeRuntimeEventSource.Keywords.ContentionKeyword) ?? false; + NativeRuntimeEventSource.Keywords.ContentionKeyword); AutoResetEvent waitEvent = _waitEvent ?? CreateWaitEvent(areContentionEventsEnabled); if (State.TryLockBeforeWait(this)) { @@ -463,7 +479,7 @@ private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId) long waitStartTimeTicks = 0; if (areContentionEventsEnabled) { - NativeRuntimeEventSource.Log!.ContentionStart(this); + NativeRuntimeEventSource.Log.ContentionStart(this); waitStartTimeTicks = Stopwatch.GetTimestamp(); } @@ -472,7 +488,7 @@ private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId) int remainingTimeoutMs = timeoutMs; while (true) { - if (!waitEvent.WaitOne(remainingTimeoutMs)) + if (!waitEvent.WaitOneNoCheck(remainingTimeoutMs, UseAlertableWaits)) { break; } @@ -535,7 +551,7 @@ private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId) { double waitDurationNs = (Stopwatch.GetTimestamp() - waitStartTimeTicks) * 1_000_000_000.0 / Stopwatch.Frequency; - NativeRuntimeEventSource.Log!.ContentionStop(waitDurationNs); + NativeRuntimeEventSource.Log.ContentionStop(waitDurationNs); } return currentThreadId; @@ -551,7 +567,15 @@ private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId) return new ThreadId(0); } - private void ResetWaiterStartTime() => _waiterStartTimeMs = 0; + private bool UseAlertableWaits => (_waiterStartTimeMsAndFlags & 1) == 0; + + private ushort WaiterStartTimeMs + { + get => (ushort)(_waiterStartTimeMsAndFlags >> 1); + set => _waiterStartTimeMsAndFlags = (ushort)((value << 1) | (_waiterStartTimeMsAndFlags & 1)); + } + + private void ResetWaiterStartTime() => WaiterStartTimeMs = 0; [MethodImpl(MethodImplOptions.AggressiveInlining)] private void RecordWaiterStartTime() @@ -562,7 +586,7 @@ private void RecordWaiterStartTime() // Don't record zero, that value is reserved for indicating that a time is not recorded currentTimeMs--; } - _waiterStartTimeMs = currentTimeMs; + WaiterStartTimeMs = currentTimeMs; } private bool ShouldStopPreemptingWaiters @@ -571,7 +595,7 @@ private bool ShouldStopPreemptingWaiters get { // If the recorded time is zero, a time has not been recorded yet - ushort waiterStartTimeMs = _waiterStartTimeMs; + ushort waiterStartTimeMs = WaiterStartTimeMs; return waiterStartTimeMs != 0 && (ushort)Environment.TickCount - waiterStartTimeMs >= MaxDurationMsForPreemptingWaiters; diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Unix.cs index 3ba1cb132e272c..f23616c143b397 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Unix.cs @@ -7,8 +7,8 @@ namespace System.Threading { public abstract partial class WaitHandle { - private static int WaitOneCore(IntPtr handle, int millisecondsTimeout) => - WaitSubsystem.Wait(handle, millisecondsTimeout, true); + private static int WaitOneCore(IntPtr handle, int millisecondsTimeout, bool alertable) => + WaitSubsystem.Wait(handle, millisecondsTimeout, alertable); private static int WaitMultipleIgnoringSyncContextCore(Span handles, bool waitAll, int millisecondsTimeout) => WaitSubsystem.Wait(handles, waitAll, millisecondsTimeout); diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Windows.cs index bae0a8f3a23c52..5af4ab131b8f42 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Windows.cs @@ -14,11 +14,11 @@ private static unsafe int WaitMultipleIgnoringSyncContextCore(Span handl { fixed (IntPtr* pHandles = &MemoryMarshal.GetReference(handles)) { - return WaitForMultipleObjectsIgnoringSyncContext(pHandles, handles.Length, waitAll, millisecondsTimeout); + return WaitForMultipleObjectsIgnoringSyncContext(pHandles, handles.Length, waitAll, millisecondsTimeout, alertable: true); } } - private static unsafe int WaitForMultipleObjectsIgnoringSyncContext(IntPtr* pHandles, int numHandles, bool waitAll, int millisecondsTimeout) + private static unsafe int WaitForMultipleObjectsIgnoringSyncContext(IntPtr* pHandles, int numHandles, bool waitAll, int millisecondsTimeout, bool alertable) { Debug.Assert(millisecondsTimeout >= -1); @@ -27,7 +27,8 @@ private static unsafe int WaitForMultipleObjectsIgnoringSyncContext(IntPtr* pHan waitAll = false; #if NATIVEAOT // TODO: reentrant wait support https://github.com/dotnet/runtime/issues/49518 - bool reentrantWait = Thread.ReentrantWaitsEnabled; + // Non-alertable waits don't allow reentrance + bool reentrantWait = alertable && Thread.ReentrantWaitsEnabled; if (reentrantWait) { @@ -92,9 +93,9 @@ private static unsafe int WaitForMultipleObjectsIgnoringSyncContext(IntPtr* pHan return result; } - internal static unsafe int WaitOneCore(IntPtr handle, int millisecondsTimeout) + internal static unsafe int WaitOneCore(IntPtr handle, int millisecondsTimeout, bool alertable) { - return WaitForMultipleObjectsIgnoringSyncContext(&handle, 1, false, millisecondsTimeout); + return WaitForMultipleObjectsIgnoringSyncContext(&handle, 1, false, millisecondsTimeout, alertable); } private static int SignalAndWaitCore(IntPtr handleToSignal, IntPtr handleToWaitOn, int millisecondsTimeout) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.cs index 58b5d8341414b3..08e5af7525f6f5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.cs @@ -106,6 +106,7 @@ public virtual bool WaitOne(int millisecondsTimeout) internal bool WaitOneNoCheck( int millisecondsTimeout, + bool alertable = true, object? associatedObject = null, NativeRuntimeEventSource.WaitHandleWaitSourceMap waitSource = NativeRuntimeEventSource.WaitHandleWaitSourceMap.Unknown) { @@ -122,22 +123,26 @@ internal bool WaitOneNoCheck( waitHandle.DangerousAddRef(ref success); int waitResult = WaitFailed; - SynchronizationContext? context = SynchronizationContext.Current; - if (context != null && context.IsWaitNotificationRequired()) + + // Check if the wait should be forwarded to a SynchronizationContext wait override. Non-alertable waits don't + // allow reentrance or interruption, and are not forwarded. + bool usedSyncContextWait = false; + if (alertable) { - waitResult = context.Wait(new[] { waitHandle.DangerousGetHandle() }, false, millisecondsTimeout); + SynchronizationContext? context = SynchronizationContext.Current; + if (context != null && context.IsWaitNotificationRequired()) + { + usedSyncContextWait = true; + waitResult = context.Wait(new[] { waitHandle.DangerousGetHandle() }, false, millisecondsTimeout); + } } - else + + if (!usedSyncContextWait) { #if !CORECLR // CoreCLR sends the wait events from the native side bool sendWaitEvents = millisecondsTimeout != 0 && -#if NATIVEAOT - // A null check is necessary in NativeAOT due to the possibility of reentrance during class - // construction, as this path can be reached through Lock. See - // https://github.com/dotnet/runtime/issues/94728 for a call stack. - NativeRuntimeEventSource.Log != null && -#endif + alertable && NativeRuntimeEventSource.Log.IsEnabled( EventLevel.Verbose, NativeRuntimeEventSource.Keywords.WaitHandleKeyword); @@ -149,7 +154,7 @@ internal bool WaitOneNoCheck( waitSource != NativeRuntimeEventSource.WaitHandleWaitSourceMap.MonitorWait; if (tryNonblockingWaitFirst) { - waitResult = WaitOneCore(waitHandle.DangerousGetHandle(), millisecondsTimeout: 0); + waitResult = WaitOneCore(waitHandle.DangerousGetHandle(), 0 /* millisecondsTimeout */, alertable); if (waitResult == WaitTimeout) { // Do a full wait and send the wait events @@ -171,7 +176,7 @@ internal bool WaitOneNoCheck( if (!tryNonblockingWaitFirst) #endif { - waitResult = WaitOneCore(waitHandle.DangerousGetHandle(), millisecondsTimeout); + waitResult = WaitOneCore(waitHandle.DangerousGetHandle(), millisecondsTimeout, alertable); } #if !CORECLR // CoreCLR sends the wait events from the native side From 6d2fa6488d4a3027cd9cc67f379b706e3dc7cfe5 Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Tue, 23 Jan 2024 09:54:05 -0800 Subject: [PATCH 2/2] Rename `useAlertableWaits` to `useTrivialWaits` --- .../src/System/Threading/WaitHandle.CoreCLR.cs | 2 +- .../Collections/Concurrent/ConcurrentUnifier.cs | 2 +- .../Collections/Concurrent/ConcurrentUnifierW.cs | 2 +- .../Concurrent/ConcurrentUnifierWKeyed.cs | 2 +- .../Internal/Runtime/FrozenObjectHeapManager.cs | 2 +- .../CompilerServices/ClassConstructorRunner.cs | 6 +++--- .../InteropServices/ComWrappers.NativeAot.cs | 2 +- .../src/System/Threading/Condition.cs | 2 +- .../src/System/Threading/SyncTable.cs | 2 +- .../System/Threading/Thread.NativeAot.Windows.cs | 2 +- .../src/System/Threading/Thread.NativeAot.cs | 2 +- .../src/System/Type.NativeAot.cs | 2 +- ...nvironment.ConstructedGenericsRegistration.cs | 2 +- .../TypeLoaderEnvironment.StaticsLookup.cs | 2 +- .../Runtime/TypeLoader/TypeLoaderEnvironment.cs | 2 +- .../TypeLoader/TypeSystemContextFactory.cs | 2 +- src/coreclr/vm/comwaithandle.cpp | 4 ++-- src/coreclr/vm/comwaithandle.h | 2 +- .../src/System/Threading/Lock.cs | 16 ++++++++++------ .../src/System/Threading/WaitHandle.Unix.cs | 4 ++-- .../src/System/Threading/WaitHandle.Windows.cs | 12 ++++++------ .../src/System/Threading/WaitHandle.cs | 14 +++++++------- 22 files changed, 46 insertions(+), 42 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/WaitHandle.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/WaitHandle.CoreCLR.cs index 02982474ddd076..b70a688e9d55d0 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Threading/WaitHandle.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/WaitHandle.CoreCLR.cs @@ -9,7 +9,7 @@ namespace System.Threading public abstract partial class WaitHandle { [MethodImpl(MethodImplOptions.InternalCall)] - private static extern int WaitOneCore(IntPtr waitHandle, int millisecondsTimeout, bool alertable); + private static extern int WaitOneCore(IntPtr waitHandle, int millisecondsTimeout, bool useTrivialWaits); private static unsafe int WaitMultipleIgnoringSyncContextCore(Span waitHandles, bool waitAll, int millisecondsTimeout) { diff --git a/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifier.cs b/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifier.cs index 4147ec0127bdd0..4e8b3ed564db5a 100644 --- a/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifier.cs +++ b/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifier.cs @@ -65,7 +65,7 @@ internal abstract class ConcurrentUnifier { protected ConcurrentUnifier() { - _lock = new Lock(useAlertableWaits: false); + _lock = new Lock(useTrivialWaits: true); _container = new Container(this); } diff --git a/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierW.cs b/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierW.cs index 80792b735a3e1c..efca6e2efaeb0a 100644 --- a/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierW.cs +++ b/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierW.cs @@ -75,7 +75,7 @@ internal abstract class ConcurrentUnifierW { protected ConcurrentUnifierW() { - _lock = new Lock(useAlertableWaits: false); + _lock = new Lock(useTrivialWaits: true); _container = new Container(this); } diff --git a/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierWKeyed.cs b/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierWKeyed.cs index 0426d2387cdd41..7cd63314c35ee8 100644 --- a/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierWKeyed.cs +++ b/src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierWKeyed.cs @@ -84,7 +84,7 @@ internal abstract class ConcurrentUnifierWKeyed { protected ConcurrentUnifierWKeyed() { - _lock = new Lock(useAlertableWaits: false); + _lock = new Lock(useTrivialWaits: true); _container = new Container(this); } diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/FrozenObjectHeapManager.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/FrozenObjectHeapManager.cs index 5782c7d6a2452d..81f1eec9cfdf2a 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/FrozenObjectHeapManager.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/FrozenObjectHeapManager.cs @@ -16,7 +16,7 @@ internal unsafe partial class FrozenObjectHeapManager { public static readonly FrozenObjectHeapManager Instance = new FrozenObjectHeapManager(); - private readonly Lock m_Crst = new Lock(useAlertableWaits: false); + private readonly Lock m_Crst = new Lock(useTrivialWaits: true); private FrozenObjectSegment m_CurrentSegment; // Default size to reserve for a frozen segment diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/ClassConstructorRunner.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/ClassConstructorRunner.cs index 5f7bfd49cf9730..c3baa5a7dad04a 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/ClassConstructorRunner.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/ClassConstructorRunner.cs @@ -275,7 +275,7 @@ public static CctorHandle GetCctor(StaticClassConstructionContext* pContext) #if TARGET_WASM if (s_cctorGlobalLock == null) { - Interlocked.CompareExchange(ref s_cctorGlobalLock, new Lock(useAlertableWaits: false), null); + Interlocked.CompareExchange(ref s_cctorGlobalLock, new Lock(useTrivialWaits: true), null); } if (s_cctorArrays == null) { @@ -342,7 +342,7 @@ public static CctorHandle GetCctor(StaticClassConstructionContext* pContext) Debug.Assert(resultArray[resultIndex]._pContext == default(StaticClassConstructionContext*)); resultArray[resultIndex]._pContext = pContext; - resultArray[resultIndex].Lock = new Lock(useAlertableWaits: false); + resultArray[resultIndex].Lock = new Lock(useTrivialWaits: true); s_count++; } @@ -489,7 +489,7 @@ public static CctorHandle GetCctorThatThreadIsBlockedOn(int managedThreadId) internal static void Initialize() { s_cctorArrays = new Cctor[10][]; - s_cctorGlobalLock = new Lock(useAlertableWaits: false); + s_cctorGlobalLock = new Lock(useTrivialWaits: true); } [Conditional("ENABLE_NOISY_CCTOR_LOG")] diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs index 6e25448024fc08..e972e1c4296fbc 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs @@ -42,7 +42,7 @@ public abstract partial class ComWrappers private static readonly List s_referenceTrackerNativeObjectWrapperCache = new List(); private readonly ConditionalWeakTable _ccwTable = new ConditionalWeakTable(); - private readonly Lock _lock = new Lock(useAlertableWaits: false); + private readonly Lock _lock = new Lock(useTrivialWaits: true); private readonly Dictionary _rcwCache = new Dictionary(); internal static bool TryGetComInstanceForIID(object obj, Guid iid, out IntPtr unknown, out long wrapperId) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Condition.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Condition.cs index c4ba1c7a1ba927..1c6cdadaf02279 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Condition.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Condition.cs @@ -114,7 +114,7 @@ public unsafe bool Wait(int millisecondsTimeout, object? associatedObjectForMoni success = waiter.ev.WaitOneNoCheck( millisecondsTimeout, - true, // alertable + false, // useTrivialWaits associatedObjectForMonitorWait, associatedObjectForMonitorWait != null ? NativeRuntimeEventSource.WaitHandleWaitSourceMap.MonitorWait diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/SyncTable.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/SyncTable.cs index 0ce15862a20197..c3a273d32739a5 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/SyncTable.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/SyncTable.cs @@ -65,7 +65,7 @@ internal static class SyncTable /// /// Protects all mutable operations on s_entries, s_freeEntryList, s_unusedEntryIndex. Also protects growing the table. /// - internal static readonly Lock s_lock = new Lock(useAlertableWaits: false); + internal static readonly Lock s_lock = new Lock(useTrivialWaits: true); /// /// The dynamically growing array of sync entries. diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs index 5e58f9f93b57a5..d6ea54412e1058 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs @@ -167,7 +167,7 @@ private bool JoinInternal(int millisecondsTimeout) } else { - result = WaitHandle.WaitOneCore(waitHandle.DangerousGetHandle(), millisecondsTimeout, alertable: true); + result = WaitHandle.WaitOneCore(waitHandle.DangerousGetHandle(), millisecondsTimeout, useTrivialWaits: false); } return result == (int)Interop.Kernel32.WAIT_OBJECT_0; diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.cs index 0cf8e0381b538a..7fc526fc8347e0 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.cs @@ -31,7 +31,7 @@ public sealed partial class Thread private Exception? _startException; // Protects starting the thread and setting its priority - private Lock _lock = new Lock(useAlertableWaits: false); + private Lock _lock = new Lock(useTrivialWaits: true); // This is used for a quick check on thread pool threads after running a work item to determine if the name, background // state, or priority were changed by the work item, and if so to reset it. Other threads may also change some of those, diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Type.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Type.NativeAot.cs index 1fc6c3535b4c0b..f9e7b2cdbd1066 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Type.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Type.NativeAot.cs @@ -42,7 +42,7 @@ internal static unsafe RuntimeType GetTypeFromMethodTable(MethodTable* pMT) private static class AllocationLockHolder { - public static Lock AllocationLock = new Lock(useAlertableWaits: false); + public static Lock AllocationLock = new Lock(useTrivialWaits: true); } [MethodImpl(MethodImplOptions.NoInlining)] diff --git a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.ConstructedGenericsRegistration.cs b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.ConstructedGenericsRegistration.cs index 794ade7a16aab9..72c65865ff2afe 100644 --- a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.ConstructedGenericsRegistration.cs +++ b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.ConstructedGenericsRegistration.cs @@ -24,7 +24,7 @@ internal struct DynamicGenericsRegistrationData } // To keep the synchronization simple, we execute all dynamic generic type registration/lookups under a global lock - private Lock _dynamicGenericsLock = new Lock(useAlertableWaits: false); + private Lock _dynamicGenericsLock = new Lock(useTrivialWaits: true); internal void RegisterDynamicGenericTypesAndMethods(DynamicGenericsRegistrationData registrationData) { diff --git a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.StaticsLookup.cs b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.StaticsLookup.cs index 8113d0da35c496..1070b50fd75b88 100644 --- a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.StaticsLookup.cs +++ b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.StaticsLookup.cs @@ -15,7 +15,7 @@ namespace Internal.Runtime.TypeLoader public sealed partial class TypeLoaderEnvironment { // To keep the synchronization simple, we execute all TLS registration/lookups under a global lock - private Lock _threadStaticsLock = new Lock(useAlertableWaits: false); + private Lock _threadStaticsLock = new Lock(useTrivialWaits: true); // Counter to keep track of generated offsets for TLS cells of dynamic types; private LowLevelDictionary _maxThreadLocalIndex = new LowLevelDictionary(); diff --git a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.cs b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.cs index eddba6a457c7af..36f2f5dee29fe8 100644 --- a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.cs +++ b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.cs @@ -145,7 +145,7 @@ internal static void Initialize() } // To keep the synchronization simple, we execute all type loading under a global lock - private Lock _typeLoaderLock = new Lock(useAlertableWaits: false); + private Lock _typeLoaderLock = new Lock(useTrivialWaits: true); public void VerifyTypeLoaderLockHeld() { diff --git a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeSystemContextFactory.cs b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeSystemContextFactory.cs index 79b2cd44d2199d..410296beaf0b90 100644 --- a/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeSystemContextFactory.cs +++ b/src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeSystemContextFactory.cs @@ -18,7 +18,7 @@ public static class TypeSystemContextFactory // This allows us to avoid recreating the type resolution context again and again, but still allows it to go away once the types are no longer being built private static GCHandle s_cachedContext = GCHandle.Alloc(null, GCHandleType.Weak); - private static Lock s_lock = new Lock(useAlertableWaits: false); + private static Lock s_lock = new Lock(useTrivialWaits: true); public static TypeSystemContext Create() { diff --git a/src/coreclr/vm/comwaithandle.cpp b/src/coreclr/vm/comwaithandle.cpp index 3c28bc407ff282..3af42e09ecdf1b 100644 --- a/src/coreclr/vm/comwaithandle.cpp +++ b/src/coreclr/vm/comwaithandle.cpp @@ -16,7 +16,7 @@ #include "excep.h" #include "comwaithandle.h" -FCIMPL3(INT32, WaitHandleNative::CorWaitOneNative, HANDLE handle, INT32 timeout, CLR_BOOL alertable) +FCIMPL3(INT32, WaitHandleNative::CorWaitOneNative, HANDLE handle, INT32 timeout, CLR_BOOL useTrivialWaits) { FCALL_CONTRACT; @@ -28,7 +28,7 @@ FCIMPL3(INT32, WaitHandleNative::CorWaitOneNative, HANDLE handle, INT32 timeout, Thread* pThread = GET_THREAD(); - WaitMode waitMode = (WaitMode)((alertable ? WaitMode_Alertable : WaitMode_None) | WaitMode_IgnoreSyncCtx); + WaitMode waitMode = (WaitMode)((!useTrivialWaits ? WaitMode_Alertable : WaitMode_None) | WaitMode_IgnoreSyncCtx); retVal = pThread->DoAppropriateWait(1, &handle, TRUE, timeout, waitMode); HELPER_METHOD_FRAME_END(); diff --git a/src/coreclr/vm/comwaithandle.h b/src/coreclr/vm/comwaithandle.h index ccab4ae1566305..c892d7aae8551c 100644 --- a/src/coreclr/vm/comwaithandle.h +++ b/src/coreclr/vm/comwaithandle.h @@ -18,7 +18,7 @@ class WaitHandleNative { public: - static FCDECL3(INT32, CorWaitOneNative, HANDLE handle, INT32 timeout, CLR_BOOL alertable); + static FCDECL3(INT32, CorWaitOneNative, HANDLE handle, INT32 timeout, CLR_BOOL useTrivialWaits); static FCDECL4(INT32, CorWaitMultipleNative, HANDLE *handleArray, INT32 numHandles, CLR_BOOL waitForAll, INT32 timeout); static FCDECL3(INT32, CorSignalAndWaitOneNative, HANDLE waitHandleSignalUNSAFE, HANDLE waitHandleWaitUNSAFE, INT32 timeout); }; diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs index ade8229eae1ffb..66cf6a03f607a1 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs @@ -39,19 +39,19 @@ public sealed partial class Lock private uint _recursionCount; private short _spinCount; - // The lowest bit is a flag, when set it indicates that the lock should not use alertable waits + // The lowest bit is a flag, when set it indicates that the lock should use trivial waits private ushort _waiterStartTimeMsAndFlags; private AutoResetEvent? _waitEvent; #if NATIVEAOT // The method needs to be public in NativeAOT so that other private libraries can access it - public Lock(bool useAlertableWaits) + public Lock(bool useTrivialWaits) #else - internal Lock(bool useAlertableWaits) + internal Lock(bool useTrivialWaits) #endif : this() { - if (!useAlertableWaits) + if (useTrivialWaits) { _waiterStartTimeMsAndFlags = 1; } @@ -488,7 +488,7 @@ private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId) int remainingTimeoutMs = timeoutMs; while (true) { - if (!waitEvent.WaitOneNoCheck(remainingTimeoutMs, UseAlertableWaits)) + if (!waitEvent.WaitOneNoCheck(remainingTimeoutMs, UseTrivialWaits)) { break; } @@ -567,7 +567,11 @@ private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId) return new ThreadId(0); } - private bool UseAlertableWaits => (_waiterStartTimeMsAndFlags & 1) == 0; + // Trivial waits are: + // - Not interruptible by Thread.Interrupt + // - Don't allow reentrance through APCs or message pumping + // - Not forwarded to SynchronizationContext wait overrides + private bool UseTrivialWaits => (_waiterStartTimeMsAndFlags & 1) != 0; private ushort WaiterStartTimeMs { diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Unix.cs index f23616c143b397..96253742b0319c 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Unix.cs @@ -7,8 +7,8 @@ namespace System.Threading { public abstract partial class WaitHandle { - private static int WaitOneCore(IntPtr handle, int millisecondsTimeout, bool alertable) => - WaitSubsystem.Wait(handle, millisecondsTimeout, alertable); + private static int WaitOneCore(IntPtr handle, int millisecondsTimeout, bool useTrivialWaits) => + WaitSubsystem.Wait(handle, millisecondsTimeout, interruptible: !useTrivialWaits); private static int WaitMultipleIgnoringSyncContextCore(Span handles, bool waitAll, int millisecondsTimeout) => WaitSubsystem.Wait(handles, waitAll, millisecondsTimeout); diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Windows.cs index 5af4ab131b8f42..42827b55256531 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Windows.cs @@ -14,11 +14,11 @@ private static unsafe int WaitMultipleIgnoringSyncContextCore(Span handl { fixed (IntPtr* pHandles = &MemoryMarshal.GetReference(handles)) { - return WaitForMultipleObjectsIgnoringSyncContext(pHandles, handles.Length, waitAll, millisecondsTimeout, alertable: true); + return WaitForMultipleObjectsIgnoringSyncContext(pHandles, handles.Length, waitAll, millisecondsTimeout, useTrivialWaits: false); } } - private static unsafe int WaitForMultipleObjectsIgnoringSyncContext(IntPtr* pHandles, int numHandles, bool waitAll, int millisecondsTimeout, bool alertable) + private static unsafe int WaitForMultipleObjectsIgnoringSyncContext(IntPtr* pHandles, int numHandles, bool waitAll, int millisecondsTimeout, bool useTrivialWaits) { Debug.Assert(millisecondsTimeout >= -1); @@ -27,8 +27,8 @@ private static unsafe int WaitForMultipleObjectsIgnoringSyncContext(IntPtr* pHan waitAll = false; #if NATIVEAOT // TODO: reentrant wait support https://github.com/dotnet/runtime/issues/49518 - // Non-alertable waits don't allow reentrance - bool reentrantWait = alertable && Thread.ReentrantWaitsEnabled; + // Trivial waits don't allow reentrance + bool reentrantWait = !useTrivialWaits && Thread.ReentrantWaitsEnabled; if (reentrantWait) { @@ -93,9 +93,9 @@ private static unsafe int WaitForMultipleObjectsIgnoringSyncContext(IntPtr* pHan return result; } - internal static unsafe int WaitOneCore(IntPtr handle, int millisecondsTimeout, bool alertable) + internal static unsafe int WaitOneCore(IntPtr handle, int millisecondsTimeout, bool useTrivialWaits) { - return WaitForMultipleObjectsIgnoringSyncContext(&handle, 1, false, millisecondsTimeout, alertable); + return WaitForMultipleObjectsIgnoringSyncContext(&handle, 1, false, millisecondsTimeout, useTrivialWaits); } private static int SignalAndWaitCore(IntPtr handleToSignal, IntPtr handleToWaitOn, int millisecondsTimeout) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.cs index 08e5af7525f6f5..21920bc39b754f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.cs @@ -106,7 +106,7 @@ public virtual bool WaitOne(int millisecondsTimeout) internal bool WaitOneNoCheck( int millisecondsTimeout, - bool alertable = true, + bool useTrivialWaits = false, object? associatedObject = null, NativeRuntimeEventSource.WaitHandleWaitSourceMap waitSource = NativeRuntimeEventSource.WaitHandleWaitSourceMap.Unknown) { @@ -124,10 +124,10 @@ internal bool WaitOneNoCheck( int waitResult = WaitFailed; - // Check if the wait should be forwarded to a SynchronizationContext wait override. Non-alertable waits don't - // allow reentrance or interruption, and are not forwarded. + // Check if the wait should be forwarded to a SynchronizationContext wait override. Trivial waits don't allow + // reentrance or interruption, and are not forwarded. bool usedSyncContextWait = false; - if (alertable) + if (!useTrivialWaits) { SynchronizationContext? context = SynchronizationContext.Current; if (context != null && context.IsWaitNotificationRequired()) @@ -142,7 +142,7 @@ internal bool WaitOneNoCheck( #if !CORECLR // CoreCLR sends the wait events from the native side bool sendWaitEvents = millisecondsTimeout != 0 && - alertable && + !useTrivialWaits && NativeRuntimeEventSource.Log.IsEnabled( EventLevel.Verbose, NativeRuntimeEventSource.Keywords.WaitHandleKeyword); @@ -154,7 +154,7 @@ internal bool WaitOneNoCheck( waitSource != NativeRuntimeEventSource.WaitHandleWaitSourceMap.MonitorWait; if (tryNonblockingWaitFirst) { - waitResult = WaitOneCore(waitHandle.DangerousGetHandle(), 0 /* millisecondsTimeout */, alertable); + waitResult = WaitOneCore(waitHandle.DangerousGetHandle(), 0 /* millisecondsTimeout */, useTrivialWaits); if (waitResult == WaitTimeout) { // Do a full wait and send the wait events @@ -176,7 +176,7 @@ internal bool WaitOneNoCheck( if (!tryNonblockingWaitFirst) #endif { - waitResult = WaitOneCore(waitHandle.DangerousGetHandle(), millisecondsTimeout, alertable); + waitResult = WaitOneCore(waitHandle.DangerousGetHandle(), millisecondsTimeout, useTrivialWaits); } #if !CORECLR // CoreCLR sends the wait events from the native side