Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Cc @kouvel for an apparent recursion in locking

This is when trying to access the gc static base of Thread in WaitForMultipleObjectsIgnoringSyncContext. There __GetGCStaticBase_S_P_CoreLib_System_Threading_Thread takes this branch, which recurses into EnsureClassConstructorRun and the into lock again, which goes into the WaitHandle again, and then gets to loading the thread cctor again.

• It's not clear why it doesn't enter the lock when using State.TryLock. It seems to not be owned not is it locked:

    [+0x000] Object           [Type: Object]
    [+0x010] _owningThreadId  : 0x0 [Type: unsigned int]
    [+0x014] _state           : 0xdc60 [Type: unsigned int]
    [+0x018] _recursionCount  : 0x0 [Type: unsigned int]
    [+0x01c] _spinCount       : -87 [Type: short]
    [+0x01e] _waiterStartTimeMs : 0xc81a [Type: unsigned short]
    [+0x008] _waitEvent       : 0x800483c3b0 [Type: S_P_CoreLib_System_Threading_AutoResetEvent &]
    [=0x7ffe147a8680] __NONGCSTATICS   [Type: __type__NONGCSTATICSS_P_CoreLib_System_Threading_Lock]

And in the dumps I don't see thread usage that would make the Interlocked.CompareExchange fail in it.

• I am not sure why getgcstatics is not returning in that branch - it might be an artifact of the dump, but the WinStore_App!S_P_CoreLib_System_Threading_Monitor::__NONGCSTATICS marker I see is 0, which seems to mean initialized.

It's not clear why it doesn't enter the lock when using State.TryLock. It seems to not be owned not is it locked

The most likely explanation is that it was locked when the code was executing. It was not locked by the time the dump got captured.

There are no other threads in this particular case that would explain convoy, but if could definitely be a "too late" situation. The dump has 9 threads, only 3 with managed code in them. The others seem not to be doing much, but this one is 3559 frames deep, mostly in lock recursion.

System.Threading.Thread has static constructor due to this line

We either need to get rid of this static constructor; or get rid of the assumption that System.Threading.Thread does not have a static constructor in the above code.

We either need to get rid of this static constructor; or get rid of the assumption that System.Threading.Thread does not have a static constructor in the above code.

We can also force the Thread's cctor to run early, before we have other threads.
That is what likely mitigates this scenario. It may be unusual to get far and not trigger Thread's cctor before having enough threads to cause contention.

ProcessorNumberSpeedCheck takes about 1 microsecond, so is fairly cheap.

We could also move the ProcessorNumberSpeedCheck on NativeAOT to some ModuleInitializer and make Thread cctor-free.

There is a benefit on CoreCLR from storing this in a readonly static and computing in a cctor, since it can become a JIT constant.

I do not think that is useful on NativeAOT, so any global location and any kind of initialization sequence would be ok. It does not even need to be computed before anything in particular (default false might be suboptimal, but not hugely so) - just running this early(ish) is ok.

My current preference would be to make s_isProcessorNumberReallyFast not readonly on NativeAOT and move initialization somewhere else, thus making Thread cctor-free.

BTW: We do not see it in our test runs since the Thread gets initialized very early in regular .exe mode before there is any multithreading. The crash can really only happen in dynamic library mode. The watson buckets above point to backgroundTaskHost.exe where the native AOT component runs in dynamic library mode.

My current preference would be to make s_isProcessorNumberReallyFast not readonly on NativeAOT

I would move private static readonly bool s_isProcessorNumberReallyFast = ProcessorIdCache.ProcessorNumberSpeedCheck(); to ProcessIdCatche.cs, but otherwise keep things same between NAOT and regular CoreCLR. Do you see any downsides with that?

Separately, we can choose to mark ProcessorIdCache with [EagerStaticClassConstruction] if we want to run its cctor early during startup, but it should not be needed for a minimal fix.

I would move private static readonly bool s_isProcessorNumberReallyFast = ProcessorIdCache.ProcessorNumberSpeedCheck(); to ProcessIdCatche.cs, but otherwise keep things same between NAOT and regular CoreCLR. Do you see any downsides with that?

Nope. I think the choice was just because GetCurrentProcessorId API lives on Thread and we wanted to optimize for common case where getting core ID is supported by CPU.

I think even with everything being AggressiveInlining it will actually work, since the static will be formally on a different class.

Separately, we can choose to mark ProcessorIdCache with [EagerStaticClassConstruction] if we want to run its cctor early during startup, but it should not be needed for a minimal fix.

I think simply moving to ProcessorIdCache will be sufficient. As long as nothing in the Lock and related machinery does GetCurrentProcessorId, which it should not.

I think one reason why we wanted the static to be on Thread is to make it likely that by the time we need to JIT ProcessorIdCache.GetCurrentProcessorId, the readonly static was already initialized, thus JIT could treat it as a constant.

It was somewhat common pattern in the past where hot readonly statics would be initialized or "touched" on some less hot, but likely to happen early code path.
With tiered compilation, this does not seem to be as useful.