[release/8.0-staging] Don't call EnsureInstanceActive under StaticBoxInitLock #108736
Conversation
|
Tagging subscribers to this area: @mangod9 |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -4200,6 +4200,9 @@ void MethodTable::AllocateRegularStaticBox(FieldDesc* pField, Object** boxedStat | |||
| MethodTable* pFieldMT = pField->GetFieldTypeHandleThrowing().GetMethodTable(); | |||
| bool hasFixedAddr = HasFixedAddressVTStatics(); | |||
|
|
|||
| // Activate any dependent modules if necessary | |||
| pFieldMT->EnsureInstanceActive(); | |||
|
|
|||
| // Taking a lock since we might come here from multiple threads/places | |||
| CrstHolder crst(GetAppDomain()->GetStaticBoxInitLock()); | |||
There was a problem hiding this comment.
NOTE: we can also just remove the lock completely here (to improve perf a bit) and assign the field via InterlockedCompareExchange pattern here, but that might lead to small memory leaks as GC won't collect a redundant allocation on NonGC heap in case of a race.
|
Friendly reminder that today 10/14 is Code Complete for the November Release. If this fix is intended to be included in that release, please make sure it's merged before 4pm PT. Otherwise, it will have to wait until next month. |
jeffschwMSFT
added
Servicing-approved
|
@EgorBo can you take a look at the CI failures? we can merge when ready |
|
@jeffschwMSFT sorry for the delay, CI is green now - ready to merge |
Fixes a customer-reported issue #108506 for .NET 8.0. It's not an issue for .NET 9.0 and .NET 10.0 since this code was reworked in #99183 (the change is too big to backport to 8.0). Thanks to @davidwrighton for investigation and the suggested fix.
Customer Impact
The issue is a race condition leading to a dead-lock in JIT. The issue is related to Wpf assemblies, and in particular to the use of C++/CLI within WPF. Notably, we're in the process of running the module constructor to initialize the static variables on one assembly, and the field type is defined in a module with a module constructor, and on a different thread we are in the process of running that module constructor, and find ourselves needing to allocate memory for the static boxes. In this case I can see that we have a lock ordering violation, where we take two locks out of order.
The fix is to simply move
pFieldMT->EnsureInstanceActive();outside of the lock we take to assignboxedStaticHandle.Regression
Testing
We don't have a repro (a race condition difficult to reproduce)
Risk
Low