From 7b5abe9e48b36c04a00152204ad4d4f7528bef32 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Thu, 4 Jul 2024 19:46:51 +0200 Subject: [PATCH 1/2] improve thread-safety and reliability of Refresh function --- .../src/WindowsBase/MS/Internal/AvTrace.cs | 68 +++++++++---------- 1 file changed, 31 insertions(+), 37 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/WindowsBase/MS/Internal/AvTrace.cs b/src/Microsoft.DotNet.Wpf/src/WindowsBase/MS/Internal/AvTrace.cs index 8bb1e7a8b41..a24e73d5d8f 100644 --- a/src/Microsoft.DotNet.Wpf/src/WindowsBase/MS/Internal/AvTrace.cs +++ b/src/Microsoft.DotNet.Wpf/src/WindowsBase/MS/Internal/AvTrace.cs @@ -51,9 +51,13 @@ public AvTrace( GetTraceSourceDelegate getTraceSourceDelegate, ClearTraceSourceD PresentationTraceSources.TraceRefresh += new TraceRefreshEventHandler(Refresh); // Fetch and cache the TraceSource from PresentationTraceSources - Initialize(); + Initialize(IsWpfTracingEnabledInRegistry()); } + /// + /// Performs thread-safe initialization of value. + /// + static AvTrace() => LoadWpfTracingSettings(); // // Refresh this trace source -- see if it needs to be enabled @@ -64,11 +68,11 @@ public AvTrace( GetTraceSourceDelegate getTraceSourceDelegate, ClearTraceSourceD public void Refresh() { - // Cause the registry to be re-read in case it's changed. - _enabledInRegistry = null; + // Re-read current WPF Trace settings from Registry in case it has changed. + bool isEnabledInRegistry = LoadWpfTracingSettings(); // Re-initialize everything - Initialize(); + Initialize(isEnabledInRegistry); } // @@ -161,12 +165,12 @@ static public void OnRefresh() // // Internal initialization // - void Initialize( ) + private void Initialize(bool isEnabledInRegistry) { // Decide if we should actually create a TraceSource instance (doing so isn't free, // so we don't want to do it if we can avoid it). - if( ShouldCreateTraceSources() ) + if(ShouldCreateTraceSources(isEnabledInRegistry)) { // Get TraceSource from the PresentationTraceSources // (this call will indirectly create the TraceSource if one doesn't already exist) @@ -175,7 +179,7 @@ void Initialize( ) // We go enabled if tracing is enabled in the registry, if // PresentationTraceSources.Refresh has been called, or if we're in the debugger // and the debugger is supposed to enable tracing. - _isEnabled = IsWpfTracingEnabledInRegistry() || _hasBeenRefreshed || _enabledByDebugger ; + _isEnabled = isEnabledInRegistry || _hasBeenRefreshed || _enabledByDebugger ; } else { @@ -194,9 +198,9 @@ void Initialize( ) // the TraceSource.) // - static private bool ShouldCreateTraceSources() + static private bool ShouldCreateTraceSources(bool isEnabledInRegistry) { - if( IsWpfTracingEnabledInRegistry() + if( isEnabledInRegistry || IsDebuggerAttached() || _hasBeenRefreshed ) @@ -207,39 +211,29 @@ static private bool ShouldCreateTraceSources() return false; } - - - /// - /// Read the registry to see if WPF tracing is allowed - /// - - static internal bool IsWpfTracingEnabledInRegistry() + /// Initializes variable from Registry and returns the value. + /// A boolean value specifying whether WPF Tracing is enabled. + internal static bool LoadWpfTracingSettings() { - // First time this is called, initialize from the registry + // Initialize from the registry + bool enabled = false; - if( _enabledInRegistry == null ) - { - bool enabled = false; + object keyValue = SecurityHelper.ReadRegistryValue(Registry.CurrentUser, + @"Software\Microsoft\Tracing\WPF", + "ManagedTracing"); - object keyValue = SecurityHelper.ReadRegistryValue( - Registry.CurrentUser, - @"Software\Microsoft\Tracing\WPF", - "ManagedTracing"); + if (keyValue is int value && value == 1) //REG_DWORD + enabled = true; - if( keyValue is int && ((int) keyValue) == 1 ) - { - enabled = true; - } - - // Update the static. Doing this last protects us from threading problems; worse case, multiple - // threads will set the same value into it. - _enabledInRegistry = enabled; - } + // Update the static value + s_enabledInRegistry = enabled; - return (bool) _enabledInRegistry; + return enabled; } - + /// Retrieves cached value to see whether WPF Tracing is enabled. + /// A boolean value specifying whether WPF Tracing is enabled. + static internal bool IsWpfTracingEnabledInRegistry() => s_enabledInRegistry; // // Check for an attached debugger. @@ -512,8 +506,8 @@ static public Type GetTypeHelper(object value) // Cache of TraceSource instance; real value resides in PresentationTraceSources. TraceSource _traceSource; - // Cache used by IsWpfTracingEnabledInRegistry - static Nullable _enabledInRegistry = null; + // Cache used by IsWpfTracingEnabledInRegistry, written by LoadWpfTracingSettings + private static volatile bool s_enabledInRegistry = false; static char[] FormatChars = new char[]{ '{', '}' }; } From 277e47fb89b6382d451021ddf79f350810fa69c3 Mon Sep 17 00:00:00 2001 From: h3xds1nz Date: Thu, 4 Jul 2024 20:07:32 +0200 Subject: [PATCH 2/2] minor formatting fix --- .../src/WindowsBase/MS/Internal/AvTrace.cs | 30 +++++++------------ 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.DotNet.Wpf/src/WindowsBase/MS/Internal/AvTrace.cs b/src/Microsoft.DotNet.Wpf/src/WindowsBase/MS/Internal/AvTrace.cs index a24e73d5d8f..2e55406f5e9 100644 --- a/src/Microsoft.DotNet.Wpf/src/WindowsBase/MS/Internal/AvTrace.cs +++ b/src/Microsoft.DotNet.Wpf/src/WindowsBase/MS/Internal/AvTrace.cs @@ -59,13 +59,13 @@ public AvTrace( GetTraceSourceDelegate getTraceSourceDelegate, ClearTraceSourceD /// static AvTrace() => LoadWpfTracingSettings(); - // - // Refresh this trace source -- see if it needs to be enabled - // because registry setting has changed or debugger is attached. - // - // To re-read the config file, call System.Diagnostics.Trace.Refresh() . - // + /// + /// Refreshes this trace source. Checks again if it has been enabled + /// either because registry setting has changed or debugger has been attached. + ///

+ /// To re-read the config file, call + /// public void Refresh() { // Re-read current WPF Trace settings from Registry in case it has changed. @@ -165,7 +165,7 @@ static public void OnRefresh() // // Internal initialization // - private void Initialize(bool isEnabledInRegistry) + private void Initialize(bool isEnabledInRegistry) { // Decide if we should actually create a TraceSource instance (doing so isn't free, // so we don't want to do it if we can avoid it). @@ -198,17 +198,9 @@ private void Initialize(bool isEnabledInRegistry) // the TraceSource.) // - static private bool ShouldCreateTraceSources(bool isEnabledInRegistry) + private static bool ShouldCreateTraceSources(bool isEnabledInRegistry) { - if( isEnabledInRegistry - || IsDebuggerAttached() - || _hasBeenRefreshed - ) - { - return true; - } - - return false; + return isEnabledInRegistry || IsDebuggerAttached() || _hasBeenRefreshed; } /// Initializes variable from Registry and returns the value. @@ -231,9 +223,9 @@ internal static bool LoadWpfTracingSettings() return enabled; } - /// Retrieves cached value to see whether WPF Tracing is enabled. + /// Retrieves cached value from to see whether WPF Tracing is enabled. /// A boolean value specifying whether WPF Tracing is enabled. - static internal bool IsWpfTracingEnabledInRegistry() => s_enabledInRegistry; + internal static bool IsWpfTracingEnabledInRegistry() => s_enabledInRegistry; // // Check for an attached debugger.