From 9c820bb9ef52ab89dee0778bc6ba84bd8d4b4fd8 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Sat, 20 Jan 2018 08:47:25 +0100 Subject: [PATCH 01/11] sync PerformanceCounterLib --- .../src/Resources/Strings.resx | 3 + .../Diagnostics/PerformanceCounterLib.cs | 215 ++++++++++-------- .../tests/PerformanceCounterCategoryTests.cs | 82 ++++++- 3 files changed, 208 insertions(+), 92 deletions(-) diff --git a/src/System.Diagnostics.PerformanceCounter/src/Resources/Strings.resx b/src/System.Diagnostics.PerformanceCounter/src/Resources/Strings.resx index 8c7c8cf25f39..1d1118f8dc1b 100644 --- a/src/System.Diagnostics.PerformanceCounter/src/Resources/Strings.resx +++ b/src/System.Diagnostics.PerformanceCounter/src/Resources/Strings.resx @@ -204,6 +204,9 @@ Cannot create or delete the Performance Category '{0}' because access is denied. + + Cannot create the Performance Category '{0}' because it is not present on Windows registry after 10 seconds. + Invalid value {1} for property {0}. diff --git a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs index f9c6c51e6bcc..1921593ef564 100644 --- a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs +++ b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs @@ -50,11 +50,13 @@ internal class PerformanceCounterLib private string _machineName; private string _perfLcid; - private Hashtable _customCategoryTable; + private static volatile Hashtable s_libraryTable; + private Hashtable _customCategoryTable; private Hashtable _categoryTable; private Hashtable _nameTable; private Hashtable _helpTable; + private readonly object _customCategoryTableLock = new Object(); private readonly object _categoryTableLock = new Object(); private readonly object _nameTableLock = new Object(); private readonly object _helpTableLock = new Object(); @@ -299,19 +301,17 @@ internal static void CloseAllLibraries() { if (s_libraryTable != null) { - foreach (PerformanceCounterLib library in s_libraryTable.Values) - library.Close(); - - s_libraryTable = null; - } - } + //race with GetPerformanceCounterLib + lock (InternalSyncObject) + { + if (s_libraryTable != null) + { + foreach (PerformanceCounterLib library in s_libraryTable.Values) + library.Close(); - internal static void CloseAllTables() - { - if (s_libraryTable != null) - { - foreach (PerformanceCounterLib library in s_libraryTable.Values) - library.CloseTables(); + s_libraryTable = null; + } + } } } @@ -320,7 +320,9 @@ internal void CloseTables() _nameTable = null; _helpTable = null; _categoryTable = null; - _customCategoryTable = null; + //race with FindCustomCategory + lock (_customCategoryTableLock) + _customCategoryTable = null; } internal void Close() @@ -638,83 +640,85 @@ internal bool FindCustomCategory(string category, out PerformanceCounterCategory RegistryKey baseKey = null; categoryType = PerformanceCounterCategoryType.Unknown; - if (_customCategoryTable == null) + //race with CloseTables + lock (_customCategoryTableLock) { - Interlocked.CompareExchange(ref _customCategoryTable, new Hashtable(StringComparer.OrdinalIgnoreCase), null); - } + if (_customCategoryTable == null) + _customCategoryTable = new Hashtable(StringComparer.OrdinalIgnoreCase); - if (_customCategoryTable.ContainsKey(category)) - { - categoryType = (PerformanceCounterCategoryType)_customCategoryTable[category]; - return true; - } - else - { - try + if (_customCategoryTable.ContainsKey(category)) { - string keyPath = ServicePath + "\\" + category + "\\Performance"; - if (_machineName == "." || string.Equals(_machineName, ComputerName, StringComparison.OrdinalIgnoreCase)) - { - key = Registry.LocalMachine.OpenSubKey(keyPath); - } - else + categoryType = (PerformanceCounterCategoryType)_customCategoryTable[category]; + return true; + } + else + { + try { - baseKey = RegistryKey.OpenRemoteBaseKey(RegistryHive.LocalMachine, "\\\\" + _machineName); - if (baseKey != null) + string keyPath = ServicePath + "\\" + category + "\\Performance"; + if (_machineName == "." || string.Equals(_machineName, ComputerName, StringComparison.OrdinalIgnoreCase)) { - try - { - key = baseKey.OpenSubKey(keyPath); - } - catch (SecurityException) + key = Registry.LocalMachine.OpenSubKey(keyPath); + } + else + { + baseKey = RegistryKey.OpenRemoteBaseKey(RegistryHive.LocalMachine, "\\\\" + _machineName); + if (baseKey != null) { - // we may not have permission to read the registry key on the remote machine. The security exception - // is thrown when RegOpenKeyEx returns ERROR_ACCESS_DENIED or ERROR_BAD_IMPERSONATION_LEVEL - // - // In this case we return an 'Unknown' category type and 'false' to indicate the category is *not* custom. - // - categoryType = PerformanceCounterCategoryType.Unknown; - _customCategoryTable[category] = categoryType; - return false; + try + { + key = baseKey.OpenSubKey(keyPath); + } + catch (SecurityException) + { + // we may not have permission to read the registry key on the remote machine. The security exception + // is thrown when RegOpenKeyEx returns ERROR_ACCESS_DENIED or ERROR_BAD_IMPERSONATION_LEVEL + // + // In this case we return an 'Unknown' category type and 'false' to indicate the category is *not* custom. + // + categoryType = PerformanceCounterCategoryType.Unknown; + _customCategoryTable[category] = categoryType; + return false; + } } } - } - if (key != null) - { - object systemDllName = key.GetValue("Library", null, RegistryValueOptions.DoNotExpandEnvironmentNames); - if (systemDllName != null && systemDllName is string - && (string.Equals((string)systemDllName, PerformanceCounterLib.PerfShimName, StringComparison.OrdinalIgnoreCase) - || ((string)systemDllName).EndsWith(PerformanceCounterLib.PerfShimFullNameSuffix, StringComparison.OrdinalIgnoreCase))) + if (key != null) { - - object isMultiInstanceObject = key.GetValue("IsMultiInstance"); - if (isMultiInstanceObject != null) + object systemDllName = key.GetValue("Library", null, RegistryValueOptions.DoNotExpandEnvironmentNames); + if (systemDllName != null && systemDllName is string + && (string.Equals((string)systemDllName, PerformanceCounterLib.PerfShimName, StringComparison.OrdinalIgnoreCase) + || ((string)systemDllName).EndsWith(PerformanceCounterLib.PerfShimFullNameSuffix, StringComparison.OrdinalIgnoreCase))) { - categoryType = (PerformanceCounterCategoryType)isMultiInstanceObject; - if (categoryType < PerformanceCounterCategoryType.Unknown || categoryType > PerformanceCounterCategoryType.MultiInstance) + + object isMultiInstanceObject = key.GetValue("IsMultiInstance"); + if (isMultiInstanceObject != null) + { + categoryType = (PerformanceCounterCategoryType)isMultiInstanceObject; + if (categoryType < PerformanceCounterCategoryType.Unknown || categoryType > PerformanceCounterCategoryType.MultiInstance) + categoryType = PerformanceCounterCategoryType.Unknown; + } + else categoryType = PerformanceCounterCategoryType.Unknown; - } - else - categoryType = PerformanceCounterCategoryType.Unknown; - object objectID = key.GetValue("First Counter"); - if (objectID != null) - { - int firstID = (int)objectID; + object objectID = key.GetValue("First Counter"); + if (objectID != null) + { + int firstID = (int)objectID; - _customCategoryTable[category] = categoryType; - return true; + _customCategoryTable[category] = categoryType; + return true; + } } } } - } - finally - { - if (key != null) - key.Close(); - if (baseKey != null) - baseKey.Close(); + finally + { + if (key != null) + key.Close(); + if (baseKey != null) + baseKey.Close(); + } } } return false; @@ -974,24 +978,22 @@ internal static PerformanceCounterLib GetPerformanceCounterLib(string machineNam machineName = (machineName == "." ? ComputerName : machineName).ToLowerInvariant(); - if (PerformanceCounterLib.s_libraryTable == null) + //race with CloseAllLibraries + lock (InternalSyncObject) { - lock (InternalSyncObject) + if (PerformanceCounterLib.s_libraryTable == null) + PerformanceCounterLib.s_libraryTable = new Hashtable(); + + string libraryKey = machineName + ":" + lcidString; + if (PerformanceCounterLib.s_libraryTable.Contains(libraryKey)) + return (PerformanceCounterLib)PerformanceCounterLib.s_libraryTable[libraryKey]; + else { - if (PerformanceCounterLib.s_libraryTable == null) - PerformanceCounterLib.s_libraryTable = new Hashtable(); + PerformanceCounterLib library = new PerformanceCounterLib(machineName, lcidString); + PerformanceCounterLib.s_libraryTable[libraryKey] = library; + return library; } } - - string libraryKey = machineName + ":" + lcidString; - if (PerformanceCounterLib.s_libraryTable.Contains(libraryKey)) - return (PerformanceCounterLib)PerformanceCounterLib.s_libraryTable[libraryKey]; - else - { - PerformanceCounterLib library = new PerformanceCounterLib(machineName, lcidString); - PerformanceCounterLib.s_libraryTable[libraryKey] = library; - return library; - } } internal byte[] GetPerformanceData(string item) @@ -1169,6 +1171,25 @@ internal static PerformanceCounterCategoryType GetCategoryType(string machine, s return categoryType; } + public static bool IsPublished(string category) + { + PerformanceCounterLib library = GetPerformanceCounterLib(".", new CultureInfo(EnglishLCID)); + if (library.CategoryTable[category] != null) + return true; + if (CultureInfo.CurrentCulture.Parent.LCID != EnglishLCID) + { + CultureInfo culture = CultureInfo.CurrentCulture; + while (culture != CultureInfo.InvariantCulture) + { + library = GetPerformanceCounterLib(".", culture); + if (library.CategoryTable[category] != null) + return true; + culture = culture.Parent; + } + } + return false; + } + internal static void RegisterCategory(string categoryName, PerformanceCounterCategoryType categoryType, string categoryHelp, CounterCreationDataCollection creationData) { try @@ -1182,8 +1203,21 @@ internal static void RegisterCategory(string categoryName, PerformanceCounterCat CreateSymbolFile(creationData); RegisterFiles(IniFilePath, false); } - CloseAllTables(); - CloseAllLibraries(); + /* + Wait some seconds for publication, sometimes registry modification is not immediatly visibile to all. + (https://msdn.microsoft.com/en-us/library/cs38wsc4%28v=vs.110%29.aspx?f=255&MSPPError=-2147217396) + Similar pattern of GetStringTable() "int waitRetries = 14; //((2^13)-1)*10ms == approximately 1.4mins" + */ + bool isPublished = false; + DateTime now = DateTime.UtcNow; + while ((DateTime.UtcNow.Subtract(now).TotalSeconds < 10)) + { + CloseAllLibraries(); + if (isPublished = IsPublished(categoryName)) + break; + } + if (!isPublished) + throw new InvalidOperationException(SR.CantCreateCategoryRegistration); } finally { @@ -1237,7 +1271,6 @@ internal static void UnregisterCategory(string categoryName) { RegisterFiles(categoryName, true); DeleteRegistryEntry(categoryName); - CloseAllTables(); CloseAllLibraries(); } } diff --git a/src/System.Diagnostics.PerformanceCounter/tests/PerformanceCounterCategoryTests.cs b/src/System.Diagnostics.PerformanceCounter/tests/PerformanceCounterCategoryTests.cs index bc50fa079e39..8df44ebf590d 100644 --- a/src/System.Diagnostics.PerformanceCounter/tests/PerformanceCounterCategoryTests.cs +++ b/src/System.Diagnostics.PerformanceCounter/tests/PerformanceCounterCategoryTests.cs @@ -1,10 +1,12 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. - +//#define MyTrait using System; using System.Collections; +using System.Collections.Generic; using System.Collections.Specialized; +using System.Threading; using Xunit; namespace System.Diagnostics.Tests @@ -12,6 +14,84 @@ namespace System.Diagnostics.Tests [SkipOnTargetFramework(TargetFrameworkMonikers.Uap)] // In appcontainer, cannot write to perf counters public static class PerformanceCounterCategoryTests { +#if MyTrait + [ConditionalFact(typeof(Helpers), nameof(Helpers.IsElevatedAndCanWriteToPerfCounters))] + [Trait("MyTrait", "MyTrait")] + public static void PerformanceCounterCategory_MyTest() + { + var pc = Environment.ProcessorCount * 8; + + Console.WriteLine("Clean counters"); + List names = new List(); + for (int i = 0; i < pc; i++) + { + var name = nameof(PerformanceCounterCategory_MyTest) + "_Counter_" + i; + if (PerformanceCounterCategory.Exists(name)) + { + PerformanceCounterCategory.Delete(name); + Console.WriteLine("Delete category {0}", name); + } + names.Add(name); + } + Console.WriteLine("End clean counters"); + Console.WriteLine($"Total counter {names.Count}"); + + Console.WriteLine($"[{DateTime.UtcNow}]Test start"); + foreach (var i in names) + { + new Thread( + catName => + { + + while (true) + { + try + { + PerformanceCounterCategory category = null; + if (PerformanceCounterCategory.Exists((string)catName)) + { + PerformanceCounterCategory.Delete((string)catName); + } + category = PerformanceCounterCategory.Create((string)catName, "description", PerformanceCounterCategoryType.SingleInstance, (string)catName, "counter description"); + var pcc = new PerformanceCounterCategory(category.CategoryName); + + try + { + DateTime dt = DateTime.Now; + while (true) + { + var counters = pcc.GetCounters(); + var value = counters[0].NextValue(); + var sample = counters[0].NextSample().RawValue; + if ((DateTime.Now - dt).Seconds > 5) + break; + + } + } + catch (Exception ex1) + { + if (ex1 is NullReferenceException) + System.Diagnostics.Debugger.Break(); + Console.WriteLine($"[{DateTime.UtcNow}]GetCounters() {ex1.Message} {catName}"); + } + + } + catch (Exception ex2) + { + if (ex2 is NullReferenceException) + System.Diagnostics.Debugger.Break(); + Console.WriteLine($"[{DateTime.UtcNow}]GetCounters() {ex2.Message} {catName}"); + } + } + + }).Start(i); + + } + + System.Threading.Thread.Sleep(Timeout.Infinite); + + } +#endif [ConditionalFact(typeof(Helpers), nameof(Helpers.IsElevatedAndCanWriteToPerfCounters))] public static void PerformanceCounterCategory_CreatePerformanceCounterCategory_DefaultConstructor() { From 6af61058ed0aea5c8b4ee51a5b58cc902d70d38a Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Thu, 22 Feb 2018 09:42:55 +0100 Subject: [PATCH 02/11] add enclosing braces --- .../src/System/Diagnostics/PerformanceCounterLib.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs index 1921593ef564..0414633d16e4 100644 --- a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs +++ b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs @@ -322,7 +322,9 @@ internal void CloseTables() _categoryTable = null; //race with FindCustomCategory lock (_customCategoryTableLock) + { _customCategoryTable = null; + } } internal void Close() @@ -644,7 +646,9 @@ internal bool FindCustomCategory(string category, out PerformanceCounterCategory lock (_customCategoryTableLock) { if (_customCategoryTable == null) + { _customCategoryTable = new Hashtable(StringComparer.OrdinalIgnoreCase); + } if (_customCategoryTable.ContainsKey(category)) { From f18a35523dbaa6f6d635e819d3a345354f511da4 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Wed, 7 Mar 2018 11:44:36 +0100 Subject: [PATCH 03/11] address PR feedback --- .../src/Resources/Strings.resx | 3 - .../tests/PerformanceCounterCategoryTests.cs | 82 +------------------ 2 files changed, 1 insertion(+), 84 deletions(-) diff --git a/src/System.Diagnostics.PerformanceCounter/src/Resources/Strings.resx b/src/System.Diagnostics.PerformanceCounter/src/Resources/Strings.resx index 1d1118f8dc1b..8c7c8cf25f39 100644 --- a/src/System.Diagnostics.PerformanceCounter/src/Resources/Strings.resx +++ b/src/System.Diagnostics.PerformanceCounter/src/Resources/Strings.resx @@ -204,9 +204,6 @@ Cannot create or delete the Performance Category '{0}' because access is denied. - - Cannot create the Performance Category '{0}' because it is not present on Windows registry after 10 seconds. - Invalid value {1} for property {0}. diff --git a/src/System.Diagnostics.PerformanceCounter/tests/PerformanceCounterCategoryTests.cs b/src/System.Diagnostics.PerformanceCounter/tests/PerformanceCounterCategoryTests.cs index 8df44ebf590d..bc50fa079e39 100644 --- a/src/System.Diagnostics.PerformanceCounter/tests/PerformanceCounterCategoryTests.cs +++ b/src/System.Diagnostics.PerformanceCounter/tests/PerformanceCounterCategoryTests.cs @@ -1,12 +1,10 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -//#define MyTrait + using System; using System.Collections; -using System.Collections.Generic; using System.Collections.Specialized; -using System.Threading; using Xunit; namespace System.Diagnostics.Tests @@ -14,84 +12,6 @@ namespace System.Diagnostics.Tests [SkipOnTargetFramework(TargetFrameworkMonikers.Uap)] // In appcontainer, cannot write to perf counters public static class PerformanceCounterCategoryTests { -#if MyTrait - [ConditionalFact(typeof(Helpers), nameof(Helpers.IsElevatedAndCanWriteToPerfCounters))] - [Trait("MyTrait", "MyTrait")] - public static void PerformanceCounterCategory_MyTest() - { - var pc = Environment.ProcessorCount * 8; - - Console.WriteLine("Clean counters"); - List names = new List(); - for (int i = 0; i < pc; i++) - { - var name = nameof(PerformanceCounterCategory_MyTest) + "_Counter_" + i; - if (PerformanceCounterCategory.Exists(name)) - { - PerformanceCounterCategory.Delete(name); - Console.WriteLine("Delete category {0}", name); - } - names.Add(name); - } - Console.WriteLine("End clean counters"); - Console.WriteLine($"Total counter {names.Count}"); - - Console.WriteLine($"[{DateTime.UtcNow}]Test start"); - foreach (var i in names) - { - new Thread( - catName => - { - - while (true) - { - try - { - PerformanceCounterCategory category = null; - if (PerformanceCounterCategory.Exists((string)catName)) - { - PerformanceCounterCategory.Delete((string)catName); - } - category = PerformanceCounterCategory.Create((string)catName, "description", PerformanceCounterCategoryType.SingleInstance, (string)catName, "counter description"); - var pcc = new PerformanceCounterCategory(category.CategoryName); - - try - { - DateTime dt = DateTime.Now; - while (true) - { - var counters = pcc.GetCounters(); - var value = counters[0].NextValue(); - var sample = counters[0].NextSample().RawValue; - if ((DateTime.Now - dt).Seconds > 5) - break; - - } - } - catch (Exception ex1) - { - if (ex1 is NullReferenceException) - System.Diagnostics.Debugger.Break(); - Console.WriteLine($"[{DateTime.UtcNow}]GetCounters() {ex1.Message} {catName}"); - } - - } - catch (Exception ex2) - { - if (ex2 is NullReferenceException) - System.Diagnostics.Debugger.Break(); - Console.WriteLine($"[{DateTime.UtcNow}]GetCounters() {ex2.Message} {catName}"); - } - } - - }).Start(i); - - } - - System.Threading.Thread.Sleep(Timeout.Infinite); - - } -#endif [ConditionalFact(typeof(Helpers), nameof(Helpers.IsElevatedAndCanWriteToPerfCounters))] public static void PerformanceCounterCategory_CreatePerformanceCounterCategory_DefaultConstructor() { From 33b961cf8880acfd5c05e8eacdd10ce9906cf290 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Wed, 7 Mar 2018 11:44:55 +0100 Subject: [PATCH 04/11] address PR feedback --- .../Diagnostics/PerformanceCounterLib.cs | 37 +------------------ 1 file changed, 2 insertions(+), 35 deletions(-) diff --git a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs index 0414633d16e4..a3064f8c33b4 100644 --- a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs +++ b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs @@ -1175,25 +1175,6 @@ internal static PerformanceCounterCategoryType GetCategoryType(string machine, s return categoryType; } - public static bool IsPublished(string category) - { - PerformanceCounterLib library = GetPerformanceCounterLib(".", new CultureInfo(EnglishLCID)); - if (library.CategoryTable[category] != null) - return true; - if (CultureInfo.CurrentCulture.Parent.LCID != EnglishLCID) - { - CultureInfo culture = CultureInfo.CurrentCulture; - while (culture != CultureInfo.InvariantCulture) - { - library = GetPerformanceCounterLib(".", culture); - if (library.CategoryTable[category] != null) - return true; - culture = culture.Parent; - } - } - return false; - } - internal static void RegisterCategory(string categoryName, PerformanceCounterCategoryType categoryType, string categoryHelp, CounterCreationDataCollection creationData) { try @@ -1206,22 +1187,8 @@ internal static void RegisterCategory(string categoryName, PerformanceCounterCat CreateIniFile(categoryName, categoryHelp, creationData, languageIds); CreateSymbolFile(creationData); RegisterFiles(IniFilePath, false); - } - /* - Wait some seconds for publication, sometimes registry modification is not immediatly visibile to all. - (https://msdn.microsoft.com/en-us/library/cs38wsc4%28v=vs.110%29.aspx?f=255&MSPPError=-2147217396) - Similar pattern of GetStringTable() "int waitRetries = 14; //((2^13)-1)*10ms == approximately 1.4mins" - */ - bool isPublished = false; - DateTime now = DateTime.UtcNow; - while ((DateTime.UtcNow.Subtract(now).TotalSeconds < 10)) - { - CloseAllLibraries(); - if (isPublished = IsPublished(categoryName)) - break; - } - if (!isPublished) - throw new InvalidOperationException(SR.CantCreateCategoryRegistration); + } + CloseAllLibraries(); } finally { From b39a24da9979f5e4e587205cbd6496b6fd1ee2d7 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Wed, 14 Mar 2018 16:06:48 +0100 Subject: [PATCH 05/11] address PR feedback --- .../Diagnostics/PerformanceCounterLib.cs | 138 +++++++++--------- 1 file changed, 65 insertions(+), 73 deletions(-) diff --git a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs index a3064f8c33b4..9743302f2623 100644 --- a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs +++ b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs @@ -56,7 +56,6 @@ internal class PerformanceCounterLib private Hashtable _categoryTable; private Hashtable _nameTable; private Hashtable _helpTable; - private readonly object _customCategoryTableLock = new Object(); private readonly object _categoryTableLock = new Object(); private readonly object _nameTableLock = new Object(); private readonly object _helpTableLock = new Object(); @@ -319,12 +318,8 @@ internal void CloseTables() { _nameTable = null; _helpTable = null; - _categoryTable = null; - //race with FindCustomCategory - lock (_customCategoryTableLock) - { - _customCategoryTable = null; - } + _categoryTable = null; + _customCategoryTable = null; } internal void Close() @@ -642,89 +637,86 @@ internal bool FindCustomCategory(string category, out PerformanceCounterCategory RegistryKey baseKey = null; categoryType = PerformanceCounterCategoryType.Unknown; - //race with CloseTables - lock (_customCategoryTableLock) - { - if (_customCategoryTable == null) - { - _customCategoryTable = new Hashtable(StringComparer.OrdinalIgnoreCase); - } + Hashtable table = + _customCategoryTable ?? + Interlocked.CompareExchange(ref _customCategoryTable, new Hashtable(StringComparer.OrdinalIgnoreCase), null) ?? + _customCategoryTable; - if (_customCategoryTable.ContainsKey(category)) - { - categoryType = (PerformanceCounterCategoryType)_customCategoryTable[category]; - return true; - } - else + if (table.ContainsKey(category)) + { + categoryType = (PerformanceCounterCategoryType)table[category]; + return true; + } + else + { + try { - try + string keyPath = ServicePath + "\\" + category + "\\Performance"; + if (_machineName == "." || string.Equals(_machineName, ComputerName, StringComparison.OrdinalIgnoreCase)) { - string keyPath = ServicePath + "\\" + category + "\\Performance"; - if (_machineName == "." || string.Equals(_machineName, ComputerName, StringComparison.OrdinalIgnoreCase)) - { - key = Registry.LocalMachine.OpenSubKey(keyPath); - } - else + key = Registry.LocalMachine.OpenSubKey(keyPath); + } + else + { + baseKey = RegistryKey.OpenRemoteBaseKey(RegistryHive.LocalMachine, "\\\\" + _machineName); + if (baseKey != null) { - baseKey = RegistryKey.OpenRemoteBaseKey(RegistryHive.LocalMachine, "\\\\" + _machineName); - if (baseKey != null) + try { - try - { - key = baseKey.OpenSubKey(keyPath); - } - catch (SecurityException) - { - // we may not have permission to read the registry key on the remote machine. The security exception - // is thrown when RegOpenKeyEx returns ERROR_ACCESS_DENIED or ERROR_BAD_IMPERSONATION_LEVEL - // - // In this case we return an 'Unknown' category type and 'false' to indicate the category is *not* custom. - // - categoryType = PerformanceCounterCategoryType.Unknown; - _customCategoryTable[category] = categoryType; - return false; - } + key = baseKey.OpenSubKey(keyPath); + } + catch (SecurityException) + { + // we may not have permission to read the registry key on the remote machine. The security exception + // is thrown when RegOpenKeyEx returns ERROR_ACCESS_DENIED or ERROR_BAD_IMPERSONATION_LEVEL + // + // In this case we return an 'Unknown' category type and 'false' to indicate the category is *not* custom. + // + categoryType = PerformanceCounterCategoryType.Unknown; + table[category] = categoryType; + return false; } } + } - if (key != null) + if (key != null) + { + object systemDllName = key.GetValue("Library", null, RegistryValueOptions.DoNotExpandEnvironmentNames); + if (systemDllName != null && systemDllName is string + && (string.Equals((string)systemDllName, PerformanceCounterLib.PerfShimName, StringComparison.OrdinalIgnoreCase) + || ((string)systemDllName).EndsWith(PerformanceCounterLib.PerfShimFullNameSuffix, StringComparison.OrdinalIgnoreCase))) { - object systemDllName = key.GetValue("Library", null, RegistryValueOptions.DoNotExpandEnvironmentNames); - if (systemDllName != null && systemDllName is string - && (string.Equals((string)systemDllName, PerformanceCounterLib.PerfShimName, StringComparison.OrdinalIgnoreCase) - || ((string)systemDllName).EndsWith(PerformanceCounterLib.PerfShimFullNameSuffix, StringComparison.OrdinalIgnoreCase))) - { - object isMultiInstanceObject = key.GetValue("IsMultiInstance"); - if (isMultiInstanceObject != null) - { - categoryType = (PerformanceCounterCategoryType)isMultiInstanceObject; - if (categoryType < PerformanceCounterCategoryType.Unknown || categoryType > PerformanceCounterCategoryType.MultiInstance) - categoryType = PerformanceCounterCategoryType.Unknown; - } - else + object isMultiInstanceObject = key.GetValue("IsMultiInstance"); + if (isMultiInstanceObject != null) + { + categoryType = (PerformanceCounterCategoryType)isMultiInstanceObject; + if (categoryType < PerformanceCounterCategoryType.Unknown || categoryType > PerformanceCounterCategoryType.MultiInstance) categoryType = PerformanceCounterCategoryType.Unknown; + } + else + categoryType = PerformanceCounterCategoryType.Unknown; - object objectID = key.GetValue("First Counter"); - if (objectID != null) - { - int firstID = (int)objectID; + object objectID = key.GetValue("First Counter"); + if (objectID != null) + { + int firstID = (int)objectID; - _customCategoryTable[category] = categoryType; - return true; - } + table[category] = categoryType; + return true; } } } - finally - { - if (key != null) - key.Close(); - if (baseKey != null) - baseKey.Close(); - } + } + finally + { + if (key != null) + key.Close(); + if (baseKey != null) + baseKey.Close(); } } + return false; } @@ -1187,7 +1179,7 @@ internal static void RegisterCategory(string categoryName, PerformanceCounterCat CreateIniFile(categoryName, categoryHelp, creationData, languageIds); CreateSymbolFile(creationData); RegisterFiles(IniFilePath, false); - } + } CloseAllLibraries(); } finally From fe0b5e392081580f6bba683ddb5e4e79c4767abf Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Wed, 14 Mar 2018 16:09:44 +0100 Subject: [PATCH 06/11] nit: trim spaces --- .../src/System/Diagnostics/PerformanceCounterLib.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs index 9743302f2623..c7af1bfa4df8 100644 --- a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs +++ b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs @@ -318,8 +318,8 @@ internal void CloseTables() { _nameTable = null; _helpTable = null; - _categoryTable = null; - _customCategoryTable = null; + _categoryTable = null; + _customCategoryTable = null; } internal void Close() From a9b3372fb5fbdc5570311797eb96322432dbfa7b Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Wed, 14 Mar 2018 16:32:46 +0100 Subject: [PATCH 07/11] address PR feedback --- .../Diagnostics/PerformanceCounterLib.cs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs index c7af1bfa4df8..e221cc289b6c 100644 --- a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs +++ b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs @@ -56,6 +56,7 @@ internal class PerformanceCounterLib private Hashtable _categoryTable; private Hashtable _nameTable; private Hashtable _helpTable; + private readonly object _customCategoryTableLock = new Object(); private readonly object _categoryTableLock = new Object(); private readonly object _nameTableLock = new Object(); private readonly object _helpTableLock = new Object(); @@ -319,7 +320,11 @@ internal void CloseTables() _nameTable = null; _helpTable = null; _categoryTable = null; - _customCategoryTable = null; + //race with FindCustomCategory + lock (_customCategoryTableLock) + { + _customCategoryTable = null; + } } internal void Close() @@ -673,7 +678,10 @@ internal bool FindCustomCategory(string category, out PerformanceCounterCategory // In this case we return an 'Unknown' category type and 'false' to indicate the category is *not* custom. // categoryType = PerformanceCounterCategoryType.Unknown; - table[category] = categoryType; + lock (_customCategoryTableLock) + { + table[category] = categoryType; + } return false; } } @@ -701,8 +709,10 @@ internal bool FindCustomCategory(string category, out PerformanceCounterCategory if (objectID != null) { int firstID = (int)objectID; - - table[category] = categoryType; + lock (_customCategoryTableLock) + { + table[category] = categoryType; + } return true; } } From b0c84b7f31db893ab6eae7f3b5ee9466536a5e17 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Fri, 16 Mar 2018 19:06:30 +0100 Subject: [PATCH 08/11] address stephentoub PR feedback --- .../src/System/Diagnostics/PerformanceCounterLib.cs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs index e221cc289b6c..acda7e4abb3a 100644 --- a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs +++ b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs @@ -55,8 +55,7 @@ internal class PerformanceCounterLib private Hashtable _customCategoryTable; private Hashtable _categoryTable; private Hashtable _nameTable; - private Hashtable _helpTable; - private readonly object _customCategoryTableLock = new Object(); + private Hashtable _helpTable; private readonly object _categoryTableLock = new Object(); private readonly object _nameTableLock = new Object(); private readonly object _helpTableLock = new Object(); @@ -320,11 +319,7 @@ internal void CloseTables() _nameTable = null; _helpTable = null; _categoryTable = null; - //race with FindCustomCategory - lock (_customCategoryTableLock) - { - _customCategoryTable = null; - } + _customCategoryTable = null; } internal void Close() @@ -678,7 +673,7 @@ internal bool FindCustomCategory(string category, out PerformanceCounterCategory // In this case we return an 'Unknown' category type and 'false' to indicate the category is *not* custom. // categoryType = PerformanceCounterCategoryType.Unknown; - lock (_customCategoryTableLock) + lock (table) { table[category] = categoryType; } @@ -709,7 +704,7 @@ internal bool FindCustomCategory(string category, out PerformanceCounterCategory if (objectID != null) { int firstID = (int)objectID; - lock (_customCategoryTableLock) + lock (table) { table[category] = categoryType; } From c24cf836fb14515d7a45a7d99d7e58f1f7695003 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Fri, 16 Mar 2018 19:07:48 +0100 Subject: [PATCH 09/11] nit: tab --- .../src/System/Diagnostics/PerformanceCounterLib.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs index acda7e4abb3a..f5b69fb93102 100644 --- a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs +++ b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs @@ -55,7 +55,7 @@ internal class PerformanceCounterLib private Hashtable _customCategoryTable; private Hashtable _categoryTable; private Hashtable _nameTable; - private Hashtable _helpTable; + private Hashtable _helpTable; private readonly object _categoryTableLock = new Object(); private readonly object _nameTableLock = new Object(); private readonly object _helpTableLock = new Object(); From eec8624da32f59beb06ccda0e647a461668c77e4 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Fri, 16 Mar 2018 19:09:37 +0100 Subject: [PATCH 10/11] nit: tab --- .../src/System/Diagnostics/PerformanceCounterLib.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs index f5b69fb93102..446c091c5449 100644 --- a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs +++ b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs @@ -319,7 +319,7 @@ internal void CloseTables() _nameTable = null; _helpTable = null; _categoryTable = null; - _customCategoryTable = null; + _customCategoryTable = null; } internal void Close() From fd0fb8588d3c73584c71a6250ae0229ea660d8ec Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Thu, 22 Mar 2018 22:32:17 +0100 Subject: [PATCH 11/11] address PR feedback --- .../src/System/Diagnostics/PerformanceCounterLib.cs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs index 446c091c5449..7c52419dc5ca 100644 --- a/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs +++ b/src/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceCounterLib.cs @@ -314,12 +314,21 @@ internal static void CloseAllLibraries() } } + internal static void CloseAllTables() + { + if (s_libraryTable != null) + { + foreach (PerformanceCounterLib library in s_libraryTable.Values) + library.CloseTables(); + } + } + internal void CloseTables() { _nameTable = null; _helpTable = null; _categoryTable = null; - _customCategoryTable = null; + _customCategoryTable = null; } internal void Close() @@ -1185,6 +1194,7 @@ internal static void RegisterCategory(string categoryName, PerformanceCounterCat CreateSymbolFile(creationData); RegisterFiles(IniFilePath, false); } + CloseAllTables(); CloseAllLibraries(); } finally @@ -1239,6 +1249,7 @@ internal static void UnregisterCategory(string categoryName) { RegisterFiles(categoryName, true); DeleteRegistryEntry(categoryName); + CloseAllTables(); CloseAllLibraries(); } }