From ee9426a4c2b43055ee43923d621c98e0f84fb5ed Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Wed, 17 Jan 2024 19:09:13 -0800 Subject: [PATCH] [NativeAOT] Fix Lock's usage of NativeRuntimeEventSource.Log to account for recursive accesses during its own class construction (#94873) * Fix Lock's usage of NativeRuntimeEventSource.Log to account for recursive accesses during its own class construction - `NativeRuntimeEventSource.Log` can be null if it's being constructed in the same thread earlier in the stack, added null checks Fixes https://github.com/dotnet/runtime/issues/94728 --- .../src/System/Threading/Lock.NativeAot.cs | 2 +- .../System.Private.CoreLib/src/System/Threading/Lock.cs | 8 ++++---- .../src/System/Threading/WaitHandle.cs | 6 ++++++ 3 files changed, 11 insertions(+), 5 deletions(-) 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 492e186b95bfc..318a8cc768024 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 @@ -173,7 +173,7 @@ private static bool TryInitializeStatics() s_minSpinCount = DetermineMinSpinCount(); // Also initialize some types that are used later to prevent potential class construction cycles - NativeRuntimeEventSource.Log.IsEnabled(); + _ = NativeRuntimeEventSource.Log; } catch { 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 fe794fdf9426e..056ff96d41b1e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs @@ -444,9 +444,9 @@ private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId) Wait: bool areContentionEventsEnabled = - NativeRuntimeEventSource.Log.IsEnabled( + NativeRuntimeEventSource.Log?.IsEnabled( EventLevel.Informational, - NativeRuntimeEventSource.Keywords.ContentionKeyword); + NativeRuntimeEventSource.Keywords.ContentionKeyword) ?? false; AutoResetEvent waitEvent = _waitEvent ?? CreateWaitEvent(areContentionEventsEnabled); if (State.TryLockBeforeWait(this)) { @@ -463,7 +463,7 @@ private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId) long waitStartTimeTicks = 0; if (areContentionEventsEnabled) { - NativeRuntimeEventSource.Log.ContentionStart(this); + NativeRuntimeEventSource.Log!.ContentionStart(this); waitStartTimeTicks = Stopwatch.GetTimestamp(); } @@ -535,7 +535,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; 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 7d78f3aea97cb..58b5d8341414b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.cs @@ -132,6 +132,12 @@ internal bool WaitOneNoCheck( #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 NativeRuntimeEventSource.Log.IsEnabled( EventLevel.Verbose, NativeRuntimeEventSource.Keywords.WaitHandleKeyword);