Address race in PerformanceCounterLib

[BruceForstall]

Error Message

Fill the error message using step by step known issues guidance.

{
  "ErrorMessage": "System.InvalidOperationException : Category 'PerformanceCounterCategory_GetCounters_Category' does not exist.",
  "BuildRetry": false,
  "ErrorPattern": "",
  "ExcludeConsoleLog": true
}

Pipeline: runtime-coreclr libraries-jitstress
net8.0-windows-Release-arm64-CoreCLR_checked-no_tiered_compilation-Windows.11.Arm64.Open
Test: System.Diagnostics.Tests.PerformanceCounterCategoryTests.PerformanceCounterCategory_GetCounters

https://dev.azure.com/dnceng-public/public/_build/results?buildId=378388&view=ms.vss-test-web.build-test-results-tab&runId=8058808&paneView=debug&resultId=124445

C:\h\w\B40D0980\w\BC6F0A05\e>"C:\h\w\B40D0980\p\dotnet.exe" exec --runtimeconfig System.Diagnostics.PerformanceCounter.Tests.runtimeconfig.json --depsfile System.Diagnostics.PerformanceCounter.Tests.deps.json xunit.console.dll System.Diagnostics.PerformanceCounter.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing  
  Discovering: System.Diagnostics.PerformanceCounter.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Diagnostics.PerformanceCounter.Tests (found 88 of 92 test cases)
  Starting:    System.Diagnostics.PerformanceCounter.Tests (parallel test collections = on, max threads = 2)

Info: Successfully installed performance counters in C:\h\w\B40D0980\t\crwb4l0j.wc1\provider.man    System.Diagnostics.Tests.PerformanceDataTests.PerformanceCounter_PerformanceData [SKIP]
      Condition(s) not met: "IsRunnableEnvironment"

Info: Successfully uninstalled the performance counters from the counter definition XML file C:\h\w\B40D0980\t\crwb4l0j.wc1\provider.man.    System.Diagnostics.Tests.PerformanceCounterCategoryTests.PerformanceCounterCategory_GetCounters [FAIL]
      System.InvalidOperationException : Category 'PerformanceCounterCategory_GetCounters_Category' does not exist.
      Stack Trace:
        /_/src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs(829,0): at System.Diagnostics.PerformanceCounterLib.GetCategorySample(String machine, String category)
        /_/src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterCategory.cs(438,0): at System.Diagnostics.PerformanceCounterCategory.GetCounterInstances(String categoryName, String machineName)
        /_/src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterCategory.cs(457,0): at System.Diagnostics.PerformanceCounterCategory.GetCounters()
        /_/src/libraries/System.Diagnostics.PerformanceCounter/tests/PerformanceCounterCategoryTests.cs(234,0): at System.Diagnostics.Tests.PerformanceCounterCategoryTests.PerformanceCounterCategory_GetCounters()
           at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
        /_/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.cs(57,0): at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
  Finished:    System.Diagnostics.PerformanceCounter.Tests
=== TEST EXECUTION SUMMARY ===
   System.Diagnostics.PerformanceCounter.Tests  Total: 92, Errors: 0, Failed: 1, Skipped: 1, Time: 20.245s

Known issue validation

Build: 🔎
Result validation: ⚠️ Validation could not be done without an Azure DevOps build URL on the issue. Please add it to the "Build: 🔎" line.
Validation performed at: 9/9/2023 1:08:12 AM UTC

Report

Summary

24-Hour Hit Count	7-Day Hit Count	1-Month Count
0	0	0

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

Issue Details

---

Pipeline: runtime-coreclr libraries-jitstress
net8.0-windows-Release-arm64-CoreCLR_checked-no_tiered_compilation-Windows.11.Arm64.Open
Test: System.Diagnostics.Tests.PerformanceCounterCategoryTests.PerformanceCounterCategory_GetCounters

https://dev.azure.com/dnceng-public/public/_build/results?buildId=378388&view=ms.vss-test-web.build-test-results-tab&runId=8058808&paneView=debug&resultId=124445

C:\h\w\B40D0980\w\BC6F0A05\e>"C:\h\w\B40D0980\p\dotnet.exe" exec --runtimeconfig System.Diagnostics.PerformanceCounter.Tests.runtimeconfig.json --depsfile System.Diagnostics.PerformanceCounter.Tests.deps.json xunit.console.dll System.Diagnostics.PerformanceCounter.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing  
  Discovering: System.Diagnostics.PerformanceCounter.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Diagnostics.PerformanceCounter.Tests (found 88 of 92 test cases)
  Starting:    System.Diagnostics.PerformanceCounter.Tests (parallel test collections = on, max threads = 2)

Info: Successfully installed performance counters in C:\h\w\B40D0980\t\crwb4l0j.wc1\provider.man    System.Diagnostics.Tests.PerformanceDataTests.PerformanceCounter_PerformanceData [SKIP]
      Condition(s) not met: "IsRunnableEnvironment"

Info: Successfully uninstalled the performance counters from the counter definition XML file C:\h\w\B40D0980\t\crwb4l0j.wc1\provider.man.    System.Diagnostics.Tests.PerformanceCounterCategoryTests.PerformanceCounterCategory_GetCounters [FAIL]
      System.InvalidOperationException : Category 'PerformanceCounterCategory_GetCounters_Category' does not exist.
      Stack Trace:
        /_/src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs(829,0): at System.Diagnostics.PerformanceCounterLib.GetCategorySample(String machine, String category)
        /_/src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterCategory.cs(438,0): at System.Diagnostics.PerformanceCounterCategory.GetCounterInstances(String categoryName, String machineName)
        /_/src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterCategory.cs(457,0): at System.Diagnostics.PerformanceCounterCategory.GetCounters()
        /_/src/libraries/System.Diagnostics.PerformanceCounter/tests/PerformanceCounterCategoryTests.cs(234,0): at System.Diagnostics.Tests.PerformanceCounterCategoryTests.PerformanceCounterCategory_GetCounters()
           at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
        /_/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.cs(57,0): at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
  Finished:    System.Diagnostics.PerformanceCounter.Tests
=== TEST EXECUTION SUMMARY ===
   System.Diagnostics.PerformanceCounter.Tests  Total: 92, Errors: 0, Failed: 1, Skipped: 1, Time: 20.245s

Author:	BruceForstall
Assignees:	-
Labels:	arch-arm64, os-windows, blocking-clean-ci-optional, area-System.Diagnostics.PerformanceCounter
Milestone:	-

[danmoseley]

the test helper that creates the perf counter category verifies it exists before returning

runtime/src/libraries/System.Diagnostics.PerformanceCounter/tests/Helpers.cs

Lines 38 to 46 in b0368ec

	public static void VerifyPerformanceCounterCategoryCreated(string categoryName)
	{
	Assert.EndsWith("_Category", categoryName);
	int tries = 0;
	while (!PerformanceCounterCategory.Exists(categoryName) && tries < 10)
	{
	System.Threading.Thread.Sleep(100);
	tries++;
	}

runtime/src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs

Lines 288 to 311 in b0368ec

	internal static bool CategoryExists(string machine, string category)
	{
	PerformanceCounterLib library = GetPerformanceCounterLib(machine, EnglishCulture);
	if (library.CategoryExists(category))
	return true;
	if (CultureInfo.CurrentCulture.Parent.Name != EnglishCulture.Name)
	{
	CultureInfo culture = CultureInfo.CurrentCulture;
	while (culture != CultureInfo.InvariantCulture)
	{
	library = GetPerformanceCounterLib(machine, culture);
	if (library.CategoryExists(category))
	return true;
	culture = culture.Parent;
	}
	}
	return false;
	}
	internal bool CategoryExists(string category)
	{
	return CategoryTable.ContainsKey(category);

yet the test later fails as it cannot find the perf counter with this category

runtime/src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs

Lines 814 to 838 in b0368ec

	PerformanceCounterLib library = GetPerformanceCounterLib(machine, EnglishCulture);
	CategorySample sample = library.GetCategorySample(category);
	if (sample == null && CultureInfo.CurrentCulture.Parent.Name != EnglishCulture.Name)
	{
	CultureInfo culture = CultureInfo.CurrentCulture;
	while (culture != CultureInfo.InvariantCulture)
	{
	library = GetPerformanceCounterLib(machine, culture);
	sample = library.GetCategorySample(category);
	if (sample != null)
	return sample;
	culture = culture.Parent;
	}
	}
	if (sample == null)
	throw new InvalidOperationException(SR.Format(SR.MissingCategory, category));
	return sample;
	}
	private CategorySample GetCategorySample(string category)
	{
	CategoryEntry entry = (CategoryEntry)CategoryTable[category];
	if (entry == null)
	return null;

either it was deleted meantime (but no other test uses a category of the same name) or one of the existence checks is faulty.

[danmoseley]

There is a cache of PerformanceCounterLib's per machine/LCID, so these are shared between concurrently running tests

runtime/src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs

Line 991 in b0368ec

if (PerformanceCounterLib.s_libraryTable.Contains(libraryKey))

each PerformanceCounterLib caches categories in a hashtable _categoryTable. It clears this eg when it is asked to create a category. Could this table be somehow stale?

incidentally, or possibly related, these two methods are called in succession, yet only one protects access to s_libraryTable against concurrent calls to GetPerformanceCounterLib(). Either both or neither should.

runtime/src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs

Lines 314 to 339 in b0368ec

	internal static void CloseAllLibraries()
	{
	if (s_libraryTable != null)
	{
	//race with GetPerformanceCounterLib
	lock (InternalSyncObject)
	{
	if (s_libraryTable != null)
	{
	foreach (PerformanceCounterLib library in s_libraryTable.Values)
	library.Close();
	s_libraryTable = null;
	}
	}
	}
	}
	internal static void CloseAllTables()
	{
	if (s_libraryTable != null)
	{
	foreach (PerformanceCounterLib library in s_libraryTable.Values)
	library.CloseTables();
	}
	}

might be useful to add some asserts and inspect the locking.

[danmoseley]

a possible race due to the above seems to be

• thread 1 retrieves CategoryTable on a PerformanceCounterLib, it's null, so it creates one, sets _categoryTable field
• thread 2 registers a category (eg), as a result the lib calls CloseAllTables(), which clears _categoryTable field
• thread 1 returns _categoryTable field which is unexpectedly null.

Looks like eg _nameTable has the same possibility.

that would result in a null ref though, not failing to find the entry.

[danmoseley]

Although the races mentioned above aren't necessarily going to cause this test failure, it makes a lot of sense to fix them before looking more at the test. Changing title and marking help wanted as this seems fairly straightforward to fix.