Avoid rare deadlocks when using TypeDescriptor

[steveharter]

Fixes#103265

Running the full suite of ComponentModel.TypeConverter.Tests (7,852 tests) results in a deadlock in a median of 1 in about of 150 cases when running locally (sample size of 4).

The cause is having two lock objects that can be locked in different orders. The fix here is to combine the locks, instead of fixing the one known case that cause a lock to be out of order compared to the other cases. Changing to a single lock avoids any other potentially unknown cases and helps prevent new cases. Combining the locks increased perf ~5% of the unit tests likely due to the same thread now only needing one lock instead of two in many scenarios; in a real-world scenario with many threads there may be a minor decrease in throughput during warmup \ startup. These lock objects are only used to add or update cache due to cache misses.

For testing, there was not a reliable way to add a unit test to trigger the rare case. With the fixes here, the verification included running the full test suite 4,000 times without a deadlock vs. ~150 times without the fix before encountering the deadlock. This was done by running a .bat file in the test artifacts folder (e.g. artifacts\bin\System.ComponentModel.TypeConverter.Tests\Release\net9.0) of the following:

@echo off
FOR /L %%A IN (1,1,2000) DO (
echo run# %%A
call ..\..\..\..\..\artifacts\bin\testhost\net9.0-windows-Release-x64\dotnet exec --runtimeconfig System.ComponentModel.TypeConverter.Tests.runtimeconfig.json --depsfile System.ComponentModel.TypeConverter.Tests.deps.json xunit.console.dll System.ComponentModel.TypeConverter.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing >result.txt
findstr /c:"Failed: 0" result.txt
if errorlevel 1 goto Fail
)
goto End
:Fail
echo FAILED
call type result.txt
:End
@echo on

The single known culprit is a call to TypeDescriptor.GetAttributes() when there is already a lock on s_internalSyncObject. This may cause a lock on TypeProvider.s_providerTable in a different lock ordering than other cases.

Sample call stacks:

// This locks one way:TypeDescriptor.GetProperties(newXElement("someElement1"));// * ReflectTypeDescriptionProvider.GetTypeData(System.Type, bool). // s_internalSyncObject lock// - ReflectTypeDescriptionProvider.IsPopulated(System.Type)// - TypeDescriptor.Refresh(System.Type)// - TypeDescriptor.AddProvider(System.ComponentModel.TypeDescriptionProvider, System.Type)// - TypeDescriptor.AddDefaultProvider(System.Type)// * TypeDescriptor.CheckDefaultProvider(System.Type). // s_providerTable lock// - TypeDescriptor.NodeFor(System.Type, bool)// - TypeDescriptor.NodeFor(System.Type)// - TypeDescriptor.NodeFor(object, bool)// - TypeDescriptor.NodeFor(object)// - TypeDescriptor.GetDescriptor(object, bool)// - TypeDescriptor.GetPropertiesImpl(object, System.Attribute[], bool, bool)// - TypeDescriptor.GetProperties(object, bool)// - TypeDescriptor.GetProperties(object)// This locks another way:usingTestComponenttestComponent=newTestComponent();testComponent.Site=newTestSiteWithService();testComponent.Disposed+=(objectobj,EventArgsargs)=>{};TypeDescriptor.GetProperties(testComponent);// * TypeDescriptor.CheckDefaultProvider(System.Type) // s_providerTable lock// - TypeDescriptor.NodeFor(System.Type, bool)// - TypeDescriptor.NodeFor(System.Type)// - TypeDescriptor.GetDescriptor(System.Type, string)// - TypeDescriptor.GetAttributes(System.Type)// - ReflectTypeDescriptionProvider.ReflectedTypeData.GetAttributes()// - ReflectTypeDescriptionProvider.GetAttributes(System.Type)// - TypeDescriptor.DefaultTypeDescriptor.GetAttributes()// - TypeDescriptor.GetAttributes(System.Type)// * ReflectTypeDescriptionProvider.ReflectGetExtendedProperties(System.ComponentModel.IExtenderProvider) // s_internalSyncObject lock// - ReflectTypeDescriptionProvider.GetExtendedProperties(object)// - TypeDescriptor.TypeDescriptionNode.DefaultExtendedTypeDescriptor.System.ComponentModel.ICustomTypeDescriptor.GetProperties()// - TypeDescriptor.GetPropertiesImpl(object, System.Attribute[], bool, bool)// - TypeDescriptor.GetProperties(object, bool)// - TypeDescriptor.GetProperties(object)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-componentmodel
See info in area-owners.md if you want to be subscribed.

[steveharter]

/azp run runtime

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[buyaa-n]

With the PR description the fix makes sense to me

[steveharter]

Fixes#103265

Running the full suite of ComponentModel.TypeConverter.Tests (7,852 tests) results in a deadlock in a median of 1 in about of 150 cases when running locally (sample size of 4).

The cause is having two lock objects that can be locked in different orders. The fix here is to combine the locks, instead of fixing the one known case that cause a lock to be out of order compared to the other cases. Changing to a single lock avoids any other potentially unknown cases and helps prevent new cases. Combining the locks increased perf ~5% of the unit tests likely due to the same thread now only needing one lock instead of two in many scenarios; in a real-world scenario with many threads there may be a minor decrease in throughput during warmup \ startup. These lock objects are only used to add or update cache due to cache misses.

For testing, there was not a reliable way to add a unit test to trigger the rare case. With the fixes here, the verification included running the full test suite 4,000 times without a deadlock vs. ~150 times without the fix before encountering the deadlock. This was done by running a .bat file in the test artifacts folder (e.g. artifacts\bin\System.ComponentModel.TypeConverter.Tests\Release\net9.0) of the following:

@echo off
FOR /L %%A IN (1,1,2000) DO (
echo run# %%A
call ..\..\..\..\..\artifacts\bin\testhost\net9.0-windows-Release-x64\dotnet exec --runtimeconfig System.ComponentModel.TypeConverter.Tests.runtimeconfig.json --depsfile System.ComponentModel.TypeConverter.Tests.deps.json xunit.console.dll System.ComponentModel.TypeConverter.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing >result.txt
findstr /c:"Failed: 0" result.txt
if errorlevel 1 goto Fail
)
goto End
:Fail
echo FAILED
call type result.txt
:End
@echo on

The single known culprit is a call to TypeDescriptor.GetAttributes() when there is already a lock on s_internalSyncObject. This may cause a lock on TypeProvider.s_providerTable in a different lock ordering than other cases.

Sample call stacks:

// This locks one way:TypeDescriptor.GetProperties(newXElement("someElement1"));// * ReflectTypeDescriptionProvider.GetTypeData(System.Type, bool). // s_internalSyncObject lock// - ReflectTypeDescriptionProvider.IsPopulated(System.Type)// - TypeDescriptor.Refresh(System.Type)// - TypeDescriptor.AddProvider(System.ComponentModel.TypeDescriptionProvider, System.Type)// - TypeDescriptor.AddDefaultProvider(System.Type)// * TypeDescriptor.CheckDefaultProvider(System.Type). // s_providerTable lock// - TypeDescriptor.NodeFor(System.Type, bool)// - TypeDescriptor.NodeFor(System.Type)// - TypeDescriptor.NodeFor(object, bool)// - TypeDescriptor.NodeFor(object)// - TypeDescriptor.GetDescriptor(object, bool)// - TypeDescriptor.GetPropertiesImpl(object, System.Attribute[], bool, bool)// - TypeDescriptor.GetProperties(object, bool)// - TypeDescriptor.GetProperties(object)// This locks another way:usingTestComponenttestComponent=newTestComponent();testComponent.Site=newTestSiteWithService();testComponent.Disposed+=(objectobj,EventArgsargs)=>{};TypeDescriptor.GetProperties(testComponent);// * TypeDescriptor.CheckDefaultProvider(System.Type) // s_providerTable lock// - TypeDescriptor.NodeFor(System.Type, bool)// - TypeDescriptor.NodeFor(System.Type)// - TypeDescriptor.GetDescriptor(System.Type, string)// - TypeDescriptor.GetAttributes(System.Type)// - ReflectTypeDescriptionProvider.ReflectedTypeData.GetAttributes()// - ReflectTypeDescriptionProvider.GetAttributes(System.Type)// - TypeDescriptor.DefaultTypeDescriptor.GetAttributes()// - TypeDescriptor.GetAttributes(System.Type)// * ReflectTypeDescriptionProvider.ReflectGetExtendedProperties(System.ComponentModel.IExtenderProvider) // s_internalSyncObject lock// - ReflectTypeDescriptionProvider.GetExtendedProperties(object)// - TypeDescriptor.TypeDescriptionNode.DefaultExtendedTypeDescriptor.System.ComponentModel.ICustomTypeDescriptor.GetProperties()// - TypeDescriptor.GetPropertiesImpl(object, System.Attribute[], bool, bool)// - TypeDescriptor.GetProperties(object, bool)// - TypeDescriptor.GetProperties(object)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-componentmodel
See info in area-owners.md if you want to be subscribed.

[steveharter]

/azp run runtime

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[buyaa-n]

With the PR description the fix makes sense to me

[steveharter]

Fixes#103265

Running the full suite of ComponentModel.TypeConverter.Tests (7,852 tests) results in a deadlock in a median of 1 in about of 150 cases when running locally (sample size of 4).

The cause is having two lock objects that can be locked in different orders. The fix here is to combine the locks, instead of fixing the one known case that cause a lock to be out of order compared to the other cases. Changing to a single lock avoids any other potentially unknown cases and helps prevent new cases. Combining the locks increased perf ~5% of the unit tests likely due to the same thread now only needing one lock instead of two in many scenarios; in a real-world scenario with many threads there may be a minor decrease in throughput during warmup \ startup. These lock objects are only used to add or update cache due to cache misses.

For testing, there was not a reliable way to add a unit test to trigger the rare case. With the fixes here, the verification included running the full test suite 4,000 times without a deadlock vs. ~150 times without the fix before encountering the deadlock. This was done by running a .bat file in the test artifacts folder (e.g. artifacts\bin\System.ComponentModel.TypeConverter.Tests\Release\net9.0) of the following:

@echo off
FOR /L %%A IN (1,1,2000) DO (
echo run# %%A
call ..\..\..\..\..\artifacts\bin\testhost\net9.0-windows-Release-x64\dotnet exec --runtimeconfig System.ComponentModel.TypeConverter.Tests.runtimeconfig.json --depsfile System.ComponentModel.TypeConverter.Tests.deps.json xunit.console.dll System.ComponentModel.TypeConverter.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing >result.txt
findstr /c:"Failed: 0" result.txt
if errorlevel 1 goto Fail
)
goto End
:Fail
echo FAILED
call type result.txt
:End
@echo on

The single known culprit is a call to TypeDescriptor.GetAttributes() when there is already a lock on s_internalSyncObject. This may cause a lock on TypeProvider.s_providerTable in a different lock ordering than other cases.

Sample call stacks:

// This locks one way:TypeDescriptor.GetProperties(newXElement("someElement1"));// * ReflectTypeDescriptionProvider.GetTypeData(System.Type, bool). // s_internalSyncObject lock// - ReflectTypeDescriptionProvider.IsPopulated(System.Type)// - TypeDescriptor.Refresh(System.Type)// - TypeDescriptor.AddProvider(System.ComponentModel.TypeDescriptionProvider, System.Type)// - TypeDescriptor.AddDefaultProvider(System.Type)// * TypeDescriptor.CheckDefaultProvider(System.Type). // s_providerTable lock// - TypeDescriptor.NodeFor(System.Type, bool)// - TypeDescriptor.NodeFor(System.Type)// - TypeDescriptor.NodeFor(object, bool)// - TypeDescriptor.NodeFor(object)// - TypeDescriptor.GetDescriptor(object, bool)// - TypeDescriptor.GetPropertiesImpl(object, System.Attribute[], bool, bool)// - TypeDescriptor.GetProperties(object, bool)// - TypeDescriptor.GetProperties(object)// This locks another way:usingTestComponenttestComponent=newTestComponent();testComponent.Site=newTestSiteWithService();testComponent.Disposed+=(objectobj,EventArgsargs)=>{};TypeDescriptor.GetProperties(testComponent);// * TypeDescriptor.CheckDefaultProvider(System.Type) // s_providerTable lock// - TypeDescriptor.NodeFor(System.Type, bool)// - TypeDescriptor.NodeFor(System.Type)// - TypeDescriptor.GetDescriptor(System.Type, string)// - TypeDescriptor.GetAttributes(System.Type)// - ReflectTypeDescriptionProvider.ReflectedTypeData.GetAttributes()// - ReflectTypeDescriptionProvider.GetAttributes(System.Type)// - TypeDescriptor.DefaultTypeDescriptor.GetAttributes()// - TypeDescriptor.GetAttributes(System.Type)// * ReflectTypeDescriptionProvider.ReflectGetExtendedProperties(System.ComponentModel.IExtenderProvider) // s_internalSyncObject lock// - ReflectTypeDescriptionProvider.GetExtendedProperties(object)// - TypeDescriptor.TypeDescriptionNode.DefaultExtendedTypeDescriptor.System.ComponentModel.ICustomTypeDescriptor.GetProperties()// - TypeDescriptor.GetPropertiesImpl(object, System.Attribute[], bool, bool)// - TypeDescriptor.GetProperties(object, bool)// - TypeDescriptor.GetProperties(object)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-componentmodel
See info in area-owners.md if you want to be subscribed.

[steveharter]

/azp run runtime

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[buyaa-n]

With the PR description the fix makes sense to me

[steveharter]

Fixes#103265

Running the full suite of ComponentModel.TypeConverter.Tests (7,852 tests) results in a deadlock in a median of 1 in about of 150 cases when running locally (sample size of 4).

The cause is having two lock objects that can be locked in different orders. The fix here is to combine the locks, instead of fixing the one known case that cause a lock to be out of order compared to the other cases. Changing to a single lock avoids any other potentially unknown cases and helps prevent new cases. Combining the locks increased perf ~5% of the unit tests likely due to the same thread now only needing one lock instead of two in many scenarios; in a real-world scenario with many threads there may be a minor decrease in throughput during warmup \ startup. These lock objects are only used to add or update cache due to cache misses.

For testing, there was not a reliable way to add a unit test to trigger the rare case. With the fixes here, the verification included running the full test suite 4,000 times without a deadlock vs. ~150 times without the fix before encountering the deadlock. This was done by running a .bat file in the test artifacts folder (e.g. artifacts\bin\System.ComponentModel.TypeConverter.Tests\Release\net9.0) of the following:

@echo off
FOR /L %%A IN (1,1,2000) DO (
echo run# %%A
call ..\..\..\..\..\artifacts\bin\testhost\net9.0-windows-Release-x64\dotnet exec --runtimeconfig System.ComponentModel.TypeConverter.Tests.runtimeconfig.json --depsfile System.ComponentModel.TypeConverter.Tests.deps.json xunit.console.dll System.ComponentModel.TypeConverter.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing >result.txt
findstr /c:"Failed: 0" result.txt
if errorlevel 1 goto Fail
)
goto End
:Fail
echo FAILED
call type result.txt
:End
@echo on

The single known culprit is a call to TypeDescriptor.GetAttributes() when there is already a lock on s_internalSyncObject. This may cause a lock on TypeProvider.s_providerTable in a different lock ordering than other cases.

Sample call stacks:

// This locks one way:TypeDescriptor.GetProperties(newXElement("someElement1"));// * ReflectTypeDescriptionProvider.GetTypeData(System.Type, bool). // s_internalSyncObject lock// - ReflectTypeDescriptionProvider.IsPopulated(System.Type)// - TypeDescriptor.Refresh(System.Type)// - TypeDescriptor.AddProvider(System.ComponentModel.TypeDescriptionProvider, System.Type)// - TypeDescriptor.AddDefaultProvider(System.Type)// * TypeDescriptor.CheckDefaultProvider(System.Type). // s_providerTable lock// - TypeDescriptor.NodeFor(System.Type, bool)// - TypeDescriptor.NodeFor(System.Type)// - TypeDescriptor.NodeFor(object, bool)// - TypeDescriptor.NodeFor(object)// - TypeDescriptor.GetDescriptor(object, bool)// - TypeDescriptor.GetPropertiesImpl(object, System.Attribute[], bool, bool)// - TypeDescriptor.GetProperties(object, bool)// - TypeDescriptor.GetProperties(object)// This locks another way:usingTestComponenttestComponent=newTestComponent();testComponent.Site=newTestSiteWithService();testComponent.Disposed+=(objectobj,EventArgsargs)=>{};TypeDescriptor.GetProperties(testComponent);// * TypeDescriptor.CheckDefaultProvider(System.Type) // s_providerTable lock// - TypeDescriptor.NodeFor(System.Type, bool)// - TypeDescriptor.NodeFor(System.Type)// - TypeDescriptor.GetDescriptor(System.Type, string)// - TypeDescriptor.GetAttributes(System.Type)// - ReflectTypeDescriptionProvider.ReflectedTypeData.GetAttributes()// - ReflectTypeDescriptionProvider.GetAttributes(System.Type)// - TypeDescriptor.DefaultTypeDescriptor.GetAttributes()// - TypeDescriptor.GetAttributes(System.Type)// * ReflectTypeDescriptionProvider.ReflectGetExtendedProperties(System.ComponentModel.IExtenderProvider) // s_internalSyncObject lock// - ReflectTypeDescriptionProvider.GetExtendedProperties(object)// - TypeDescriptor.TypeDescriptionNode.DefaultExtendedTypeDescriptor.System.ComponentModel.ICustomTypeDescriptor.GetProperties()// - TypeDescriptor.GetPropertiesImpl(object, System.Attribute[], bool, bool)// - TypeDescriptor.GetProperties(object, bool)// - TypeDescriptor.GetProperties(object)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-componentmodel
See info in area-owners.md if you want to be subscribed.

[steveharter]

/azp run runtime

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[buyaa-n]

With the PR description the fix makes sense to me

[steveharter]

Fixes#103265

Running the full suite of ComponentModel.TypeConverter.Tests (7,852 tests) results in a deadlock in a median of 1 in about of 150 cases when running locally (sample size of 4).

The cause is having two lock objects that can be locked in different orders. The fix here is to combine the locks, instead of fixing the one known case that cause a lock to be out of order compared to the other cases. Changing to a single lock avoids any other potentially unknown cases and helps prevent new cases. Combining the locks increased perf ~5% of the unit tests likely due to the same thread now only needing one lock instead of two in many scenarios; in a real-world scenario with many threads there may be a minor decrease in throughput during warmup \ startup. These lock objects are only used to add or update cache due to cache misses.

For testing, there was not a reliable way to add a unit test to trigger the rare case. With the fixes here, the verification included running the full test suite 4,000 times without a deadlock vs. ~150 times without the fix before encountering the deadlock. This was done by running a .bat file in the test artifacts folder (e.g. artifacts\bin\System.ComponentModel.TypeConverter.Tests\Release\net9.0) of the following:

@echo off
FOR /L %%A IN (1,1,2000) DO (
echo run# %%A
call ..\..\..\..\..\artifacts\bin\testhost\net9.0-windows-Release-x64\dotnet exec --runtimeconfig System.ComponentModel.TypeConverter.Tests.runtimeconfig.json --depsfile System.ComponentModel.TypeConverter.Tests.deps.json xunit.console.dll System.ComponentModel.TypeConverter.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing >result.txt
findstr /c:"Failed: 0" result.txt
if errorlevel 1 goto Fail
)
goto End
:Fail
echo FAILED
call type result.txt
:End
@echo on

The single known culprit is a call to TypeDescriptor.GetAttributes() when there is already a lock on s_internalSyncObject. This may cause a lock on TypeProvider.s_providerTable in a different lock ordering than other cases.

Sample call stacks:

// This locks one way:TypeDescriptor.GetProperties(newXElement("someElement1"));// * ReflectTypeDescriptionProvider.GetTypeData(System.Type, bool). // s_internalSyncObject lock// - ReflectTypeDescriptionProvider.IsPopulated(System.Type)// - TypeDescriptor.Refresh(System.Type)// - TypeDescriptor.AddProvider(System.ComponentModel.TypeDescriptionProvider, System.Type)// - TypeDescriptor.AddDefaultProvider(System.Type)// * TypeDescriptor.CheckDefaultProvider(System.Type). // s_providerTable lock// - TypeDescriptor.NodeFor(System.Type, bool)// - TypeDescriptor.NodeFor(System.Type)// - TypeDescriptor.NodeFor(object, bool)// - TypeDescriptor.NodeFor(object)// - TypeDescriptor.GetDescriptor(object, bool)// - TypeDescriptor.GetPropertiesImpl(object, System.Attribute[], bool, bool)// - TypeDescriptor.GetProperties(object, bool)// - TypeDescriptor.GetProperties(object)// This locks another way:usingTestComponenttestComponent=newTestComponent();testComponent.Site=newTestSiteWithService();testComponent.Disposed+=(objectobj,EventArgsargs)=>{};TypeDescriptor.GetProperties(testComponent);// * TypeDescriptor.CheckDefaultProvider(System.Type) // s_providerTable lock// - TypeDescriptor.NodeFor(System.Type, bool)// - TypeDescriptor.NodeFor(System.Type)// - TypeDescriptor.GetDescriptor(System.Type, string)// - TypeDescriptor.GetAttributes(System.Type)// - ReflectTypeDescriptionProvider.ReflectedTypeData.GetAttributes()// - ReflectTypeDescriptionProvider.GetAttributes(System.Type)// - TypeDescriptor.DefaultTypeDescriptor.GetAttributes()// - TypeDescriptor.GetAttributes(System.Type)// * ReflectTypeDescriptionProvider.ReflectGetExtendedProperties(System.ComponentModel.IExtenderProvider) // s_internalSyncObject lock// - ReflectTypeDescriptionProvider.GetExtendedProperties(object)// - TypeDescriptor.TypeDescriptionNode.DefaultExtendedTypeDescriptor.System.ComponentModel.ICustomTypeDescriptor.GetProperties()// - TypeDescriptor.GetPropertiesImpl(object, System.Attribute[], bool, bool)// - TypeDescriptor.GetProperties(object, bool)// - TypeDescriptor.GetProperties(object)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-componentmodel
See info in area-owners.md if you want to be subscribed.

[steveharter]

/azp run runtime

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[buyaa-n]

With the PR description the fix makes sense to me

[steveharter]

Fixes#103265

Running the full suite of ComponentModel.TypeConverter.Tests (7,852 tests) results in a deadlock in a median of 1 in about of 150 cases when running locally (sample size of 4).

The cause is having two lock objects that can be locked in different orders. The fix here is to combine the locks, instead of fixing the one known case that cause a lock to be out of order compared to the other cases. Changing to a single lock avoids any other potentially unknown cases and helps prevent new cases. Combining the locks increased perf ~5% of the unit tests likely due to the same thread now only needing one lock instead of two in many scenarios; in a real-world scenario with many threads there may be a minor decrease in throughput during warmup \ startup. These lock objects are only used to add or update cache due to cache misses.

For testing, there was not a reliable way to add a unit test to trigger the rare case. With the fixes here, the verification included running the full test suite 4,000 times without a deadlock vs. ~150 times without the fix before encountering the deadlock. This was done by running a .bat file in the test artifacts folder (e.g. artifacts\bin\System.ComponentModel.TypeConverter.Tests\Release\net9.0) of the following:

@echo off
FOR /L %%A IN (1,1,2000) DO (
echo run# %%A
call ..\..\..\..\..\artifacts\bin\testhost\net9.0-windows-Release-x64\dotnet exec --runtimeconfig System.ComponentModel.TypeConverter.Tests.runtimeconfig.json --depsfile System.ComponentModel.TypeConverter.Tests.deps.json xunit.console.dll System.ComponentModel.TypeConverter.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing >result.txt
findstr /c:"Failed: 0" result.txt
if errorlevel 1 goto Fail
)
goto End
:Fail
echo FAILED
call type result.txt
:End
@echo on

The single known culprit is a call to TypeDescriptor.GetAttributes() when there is already a lock on s_internalSyncObject. This may cause a lock on TypeProvider.s_providerTable in a different lock ordering than other cases.

Sample call stacks:

// This locks one way:TypeDescriptor.GetProperties(newXElement("someElement1"));// * ReflectTypeDescriptionProvider.GetTypeData(System.Type, bool). // s_internalSyncObject lock// - ReflectTypeDescriptionProvider.IsPopulated(System.Type)// - TypeDescriptor.Refresh(System.Type)// - TypeDescriptor.AddProvider(System.ComponentModel.TypeDescriptionProvider, System.Type)// - TypeDescriptor.AddDefaultProvider(System.Type)// * TypeDescriptor.CheckDefaultProvider(System.Type). // s_providerTable lock// - TypeDescriptor.NodeFor(System.Type, bool)// - TypeDescriptor.NodeFor(System.Type)// - TypeDescriptor.NodeFor(object, bool)// - TypeDescriptor.NodeFor(object)// - TypeDescriptor.GetDescriptor(object, bool)// - TypeDescriptor.GetPropertiesImpl(object, System.Attribute[], bool, bool)// - TypeDescriptor.GetProperties(object, bool)// - TypeDescriptor.GetProperties(object)// This locks another way:usingTestComponenttestComponent=newTestComponent();testComponent.Site=newTestSiteWithService();testComponent.Disposed+=(objectobj,EventArgsargs)=>{};TypeDescriptor.GetProperties(testComponent);// * TypeDescriptor.CheckDefaultProvider(System.Type) // s_providerTable lock// - TypeDescriptor.NodeFor(System.Type, bool)// - TypeDescriptor.NodeFor(System.Type)// - TypeDescriptor.GetDescriptor(System.Type, string)// - TypeDescriptor.GetAttributes(System.Type)// - ReflectTypeDescriptionProvider.ReflectedTypeData.GetAttributes()// - ReflectTypeDescriptionProvider.GetAttributes(System.Type)// - TypeDescriptor.DefaultTypeDescriptor.GetAttributes()// - TypeDescriptor.GetAttributes(System.Type)// * ReflectTypeDescriptionProvider.ReflectGetExtendedProperties(System.ComponentModel.IExtenderProvider) // s_internalSyncObject lock// - ReflectTypeDescriptionProvider.GetExtendedProperties(object)// - TypeDescriptor.TypeDescriptionNode.DefaultExtendedTypeDescriptor.System.ComponentModel.ICustomTypeDescriptor.GetProperties()// - TypeDescriptor.GetPropertiesImpl(object, System.Attribute[], bool, bool)// - TypeDescriptor.GetProperties(object, bool)// - TypeDescriptor.GetProperties(object)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-componentmodel
See info in area-owners.md if you want to be subscribed.

[steveharter]

/azp run runtime

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[buyaa-n]

With the PR description the fix makes sense to me

[steveharter]

Fixes#103265

Running the full suite of ComponentModel.TypeConverter.Tests (7,852 tests) results in a deadlock in a median of 1 in about of 150 cases when running locally (sample size of 4).

The cause is having two lock objects that can be locked in different orders. The fix here is to combine the locks, instead of fixing the one known case that cause a lock to be out of order compared to the other cases. Changing to a single lock avoids any other potentially unknown cases and helps prevent new cases. Combining the locks increased perf ~5% of the unit tests likely due to the same thread now only needing one lock instead of two in many scenarios; in a real-world scenario with many threads there may be a minor decrease in throughput during warmup \ startup. These lock objects are only used to add or update cache due to cache misses.

For testing, there was not a reliable way to add a unit test to trigger the rare case. With the fixes here, the verification included running the full test suite 4,000 times without a deadlock vs. ~150 times without the fix before encountering the deadlock. This was done by running a .bat file in the test artifacts folder (e.g. artifacts\bin\System.ComponentModel.TypeConverter.Tests\Release\net9.0) of the following:

@echo off
FOR /L %%A IN (1,1,2000) DO (
echo run# %%A
call ..\..\..\..\..\artifacts\bin\testhost\net9.0-windows-Release-x64\dotnet exec --runtimeconfig System.ComponentModel.TypeConverter.Tests.runtimeconfig.json --depsfile System.ComponentModel.TypeConverter.Tests.deps.json xunit.console.dll System.ComponentModel.TypeConverter.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing >result.txt
findstr /c:"Failed: 0" result.txt
if errorlevel 1 goto Fail
)
goto End
:Fail
echo FAILED
call type result.txt
:End
@echo on

The single known culprit is a call to TypeDescriptor.GetAttributes() when there is already a lock on s_internalSyncObject. This may cause a lock on TypeProvider.s_providerTable in a different lock ordering than other cases.

Sample call stacks:

// This locks one way:TypeDescriptor.GetProperties(newXElement("someElement1"));// * ReflectTypeDescriptionProvider.GetTypeData(System.Type, bool). // s_internalSyncObject lock// - ReflectTypeDescriptionProvider.IsPopulated(System.Type)// - TypeDescriptor.Refresh(System.Type)// - TypeDescriptor.AddProvider(System.ComponentModel.TypeDescriptionProvider, System.Type)// - TypeDescriptor.AddDefaultProvider(System.Type)// * TypeDescriptor.CheckDefaultProvider(System.Type). // s_providerTable lock// - TypeDescriptor.NodeFor(System.Type, bool)// - TypeDescriptor.NodeFor(System.Type)// - TypeDescriptor.NodeFor(object, bool)// - TypeDescriptor.NodeFor(object)// - TypeDescriptor.GetDescriptor(object, bool)// - TypeDescriptor.GetPropertiesImpl(object, System.Attribute[], bool, bool)// - TypeDescriptor.GetProperties(object, bool)// - TypeDescriptor.GetProperties(object)// This locks another way:usingTestComponenttestComponent=newTestComponent();testComponent.Site=newTestSiteWithService();testComponent.Disposed+=(objectobj,EventArgsargs)=>{};TypeDescriptor.GetProperties(testComponent);// * TypeDescriptor.CheckDefaultProvider(System.Type) // s_providerTable lock// - TypeDescriptor.NodeFor(System.Type, bool)// - TypeDescriptor.NodeFor(System.Type)// - TypeDescriptor.GetDescriptor(System.Type, string)// - TypeDescriptor.GetAttributes(System.Type)// - ReflectTypeDescriptionProvider.ReflectedTypeData.GetAttributes()// - ReflectTypeDescriptionProvider.GetAttributes(System.Type)// - TypeDescriptor.DefaultTypeDescriptor.GetAttributes()// - TypeDescriptor.GetAttributes(System.Type)// * ReflectTypeDescriptionProvider.ReflectGetExtendedProperties(System.ComponentModel.IExtenderProvider) // s_internalSyncObject lock// - ReflectTypeDescriptionProvider.GetExtendedProperties(object)// - TypeDescriptor.TypeDescriptionNode.DefaultExtendedTypeDescriptor.System.ComponentModel.ICustomTypeDescriptor.GetProperties()// - TypeDescriptor.GetPropertiesImpl(object, System.Attribute[], bool, bool)// - TypeDescriptor.GetProperties(object, bool)// - TypeDescriptor.GetProperties(object)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-componentmodel
See info in area-owners.md if you want to be subscribed.

[steveharter]

/azp run runtime

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[buyaa-n]

With the PR description the fix makes sense to me

[steveharter]

Fixes#103265

Running the full suite of ComponentModel.TypeConverter.Tests (7,852 tests) results in a deadlock in a median of 1 in about of 150 cases when running locally (sample size of 4).

The cause is having two lock objects that can be locked in different orders. The fix here is to combine the locks, instead of fixing the one known case that cause a lock to be out of order compared to the other cases. Changing to a single lock avoids any other potentially unknown cases and helps prevent new cases. Combining the locks increased perf ~5% of the unit tests likely due to the same thread now only needing one lock instead of two in many scenarios; in a real-world scenario with many threads there may be a minor decrease in throughput during warmup \ startup. These lock objects are only used to add or update cache due to cache misses.

For testing, there was not a reliable way to add a unit test to trigger the rare case. With the fixes here, the verification included running the full test suite 4,000 times without a deadlock vs. ~150 times without the fix before encountering the deadlock. This was done by running a .bat file in the test artifacts folder (e.g. artifacts\bin\System.ComponentModel.TypeConverter.Tests\Release\net9.0) of the following:

@echo off
FOR /L %%A IN (1,1,2000) DO (
echo run# %%A
call ..\..\..\..\..\artifacts\bin\testhost\net9.0-windows-Release-x64\dotnet exec --runtimeconfig System.ComponentModel.TypeConverter.Tests.runtimeconfig.json --depsfile System.ComponentModel.TypeConverter.Tests.deps.json xunit.console.dll System.ComponentModel.TypeConverter.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing >result.txt
findstr /c:"Failed: 0" result.txt
if errorlevel 1 goto Fail
)
goto End
:Fail
echo FAILED
call type result.txt
:End
@echo on

The single known culprit is a call to TypeDescriptor.GetAttributes() when there is already a lock on s_internalSyncObject. This may cause a lock on TypeProvider.s_providerTable in a different lock ordering than other cases.

Sample call stacks:

// This locks one way:TypeDescriptor.GetProperties(newXElement("someElement1"));// * ReflectTypeDescriptionProvider.GetTypeData(System.Type, bool). // s_internalSyncObject lock// - ReflectTypeDescriptionProvider.IsPopulated(System.Type)// - TypeDescriptor.Refresh(System.Type)// - TypeDescriptor.AddProvider(System.ComponentModel.TypeDescriptionProvider, System.Type)// - TypeDescriptor.AddDefaultProvider(System.Type)// * TypeDescriptor.CheckDefaultProvider(System.Type). // s_providerTable lock// - TypeDescriptor.NodeFor(System.Type, bool)// - TypeDescriptor.NodeFor(System.Type)// - TypeDescriptor.NodeFor(object, bool)// - TypeDescriptor.NodeFor(object)// - TypeDescriptor.GetDescriptor(object, bool)// - TypeDescriptor.GetPropertiesImpl(object, System.Attribute[], bool, bool)// - TypeDescriptor.GetProperties(object, bool)// - TypeDescriptor.GetProperties(object)// This locks another way:usingTestComponenttestComponent=newTestComponent();testComponent.Site=newTestSiteWithService();testComponent.Disposed+=(objectobj,EventArgsargs)=>{};TypeDescriptor.GetProperties(testComponent);// * TypeDescriptor.CheckDefaultProvider(System.Type) // s_providerTable lock// - TypeDescriptor.NodeFor(System.Type, bool)// - TypeDescriptor.NodeFor(System.Type)// - TypeDescriptor.GetDescriptor(System.Type, string)// - TypeDescriptor.GetAttributes(System.Type)// - ReflectTypeDescriptionProvider.ReflectedTypeData.GetAttributes()// - ReflectTypeDescriptionProvider.GetAttributes(System.Type)// - TypeDescriptor.DefaultTypeDescriptor.GetAttributes()// - TypeDescriptor.GetAttributes(System.Type)// * ReflectTypeDescriptionProvider.ReflectGetExtendedProperties(System.ComponentModel.IExtenderProvider) // s_internalSyncObject lock// - ReflectTypeDescriptionProvider.GetExtendedProperties(object)// - TypeDescriptor.TypeDescriptionNode.DefaultExtendedTypeDescriptor.System.ComponentModel.ICustomTypeDescriptor.GetProperties()// - TypeDescriptor.GetPropertiesImpl(object, System.Attribute[], bool, bool)// - TypeDescriptor.GetProperties(object, bool)// - TypeDescriptor.GetProperties(object)

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-componentmodel
See info in area-owners.md if you want to be subscribed.

[steveharter]

/azp run runtime

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[buyaa-n]

With the PR description the fix makes sense to me