Skip to content

Fix InvalidOperationException in AvTrace and improve general thread-safety #9352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
h3xds1nz wants to merge 2 commits into
base: main
Choose a base branch
from
+37 −51
Open

Fix InvalidOperationException in AvTrace and improve general thread-safety #9352

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7b5abe9
improve thread-safety and reliability of Refresh function
h3xds1nz Jul 4, 2024
277e47f
minor formatting fix
h3xds1nz Jul 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 37 additions & 51 deletions src/Microsoft.DotNet.Wpf/src/WindowsBase/MS/Internal/AvTrace.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,24 +51,28 @@ public AvTrace( GetTraceSourceDelegate getTraceSourceDelegate, ClearTraceSourceD
PresentationTraceSources.TraceRefresh += new TraceRefreshEventHandler(Refresh);

// Fetch and cache the TraceSource from PresentationTraceSources
Initialize();
Initialize(IsWpfTracingEnabledInRegistry());
}

/// <summary>
/// Performs thread-safe initialization of <see cref="s_enabledInRegistry"/> value.
/// </summary>
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() .
//

/// <summary>
/// Refreshes this trace source. Checks again if it has been enabled
/// either because registry setting has changed or debugger has been attached.
/// <br/><br/>
/// To re-read the config file, call <see cref="System.Diagnostics.Trace.Refresh"/>
/// </summary>
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);
}

//
Expand Down Expand Up @@ -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)
Expand All @@ -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
{
Expand All @@ -194,52 +198,34 @@ void Initialize( )
// the TraceSource.)
//

static private bool ShouldCreateTraceSources()
private static bool ShouldCreateTraceSources(bool isEnabledInRegistry)
{
if( IsWpfTracingEnabledInRegistry()
|| IsDebuggerAttached()
|| _hasBeenRefreshed
)
{
return true;
}

return false;
return isEnabledInRegistry || IsDebuggerAttached() || _hasBeenRefreshed;
}



///
/// Read the registry to see if WPF tracing is allowed
///

static internal bool IsWpfTracingEnabledInRegistry()
/// <summary> Initializes <see cref="s_enabledInRegistry"/> variable from Registry and returns the value. </summary>
/// <returns> A boolean value specifying whether WPF Tracing is enabled. </returns>
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 value
s_enabledInRegistry = enabled;

// Update the static. Doing this last protects us from threading problems; worse case, multiple
// threads will set the same value into it.
_enabledInRegistry = enabled;
}

return (bool) _enabledInRegistry;
return enabled;
}


/// <summary> Retrieves cached value from <see cref=" s_enabledInRegistry"/> to see whether WPF Tracing is enabled. </summary>
/// <returns> A boolean value specifying whether WPF Tracing is enabled. </returns>
internal static bool IsWpfTracingEnabledInRegistry() => s_enabledInRegistry;
Comment thread
h3xds1nz marked this conversation as resolved.

//
// Check for an attached debugger.
Expand Down Expand Up @@ -512,8 +498,8 @@ static public Type GetTypeHelper(object value)
// Cache of TraceSource instance; real value resides in PresentationTraceSources.
TraceSource _traceSource;

// Cache used by IsWpfTracingEnabledInRegistry
static Nullable<bool> _enabledInRegistry = null;
// Cache used by IsWpfTracingEnabledInRegistry, written by LoadWpfTracingSettings
private static volatile bool s_enabledInRegistry = false;

static char[] FormatChars = new char[]{ '{', '}' };
}
Expand Down