From 919329e524eb408321c1cd0fa86be3f7ce4d2865 Mon Sep 17 00:00:00 2001 From: Jonathan Gilbert Date: Wed, 1 Oct 2025 17:58:59 -0500 Subject: [PATCH 01/10] Changed all cases where `int` (or in one case `long`) was used instead of `bool` because of the lack of support in Interlocked.Exchange/CompareExchange to use the now-supported type, but with backwards-compatible support for .NET pre-9.0. --- .../SentryDatabaseLogging.cs | 14 +++++++++-- src/Sentry.Profiling/SampleProfilerSession.cs | 21 ++++++++++++----- .../SamplingTransactionProfilerFactory.cs | 13 ++++++++--- .../DetectBlockingSynchronizationContext.cs | 4 ++-- src/Sentry/Internal/Hub.cs | 23 +++++++++++++++---- src/Sentry/Threading/ScopedCountdownLock.cs | 21 +++++++++++++---- src/Sentry/TransactionTracer.cs | 17 +++++++++++--- 7 files changed, 88 insertions(+), 25 deletions(-) diff --git a/src/Sentry.EntityFramework/SentryDatabaseLogging.cs b/src/Sentry.EntityFramework/SentryDatabaseLogging.cs index c61ff996d2..d7855fa1cb 100644 --- a/src/Sentry.EntityFramework/SentryDatabaseLogging.cs +++ b/src/Sentry.EntityFramework/SentryDatabaseLogging.cs @@ -5,14 +5,24 @@ namespace Sentry.EntityFramework; /// internal static class SentryDatabaseLogging { - private static int Init; +#if NET9_0_OR_GREATER + private static bool _init; + + const bool TRUE = true; + const bool FALSE = false; +#else + private static int _init; + + const int TRUE = 1; + const int FALSE = 0; +#endif internal static SentryCommandInterceptor? UseBreadcrumbs( IQueryLogger? queryLogger = null, bool initOnce = true, IDiagnosticLogger? diagnosticLogger = null) { - if (initOnce && Interlocked.Exchange(ref Init, 1) != 0) + if (initOnce && Interlocked.Exchange(ref _init, TRUE) != FALSE) { diagnosticLogger?.LogWarning("{0}.{1} was already executed.", nameof(SentryDatabaseLogging), nameof(UseBreadcrumbs)); diff --git a/src/Sentry.Profiling/SampleProfilerSession.cs b/src/Sentry.Profiling/SampleProfilerSession.cs index 6cd9a9242a..ee7562cb46 100644 --- a/src/Sentry.Profiling/SampleProfilerSession.cs +++ b/src/Sentry.Profiling/SampleProfilerSession.cs @@ -56,18 +56,27 @@ private SampleProfilerSession(SentryStopwatch stopwatch, EventPipeSession sessio public TraceLog TraceLog => EventSource.TraceLog; - // default is false, set 1 for true. - private static int _throwOnNextStartupForTests = 0; +#if NET9_0_OR_GREATER + private static bool _throwOnNextStartupForTests = FALSE; + + const bool TRUE = true; + const bool FALSE = false; +#else + private static int _throwOnNextStartupForTests = FALSE; + + const int TRUE = 1; + const int FALSE = 0; +#endif internal static bool ThrowOnNextStartupForTests { - get { return Interlocked.CompareExchange(ref _throwOnNextStartupForTests, 1, 1) == 1; } + get { return _throwOnNextStartupForTests != FALSE; } set { if (value) - Interlocked.CompareExchange(ref _throwOnNextStartupForTests, 1, 0); + Interlocked.CompareExchange(ref _throwOnNextStartupForTests, TRUE, FALSE); else - Interlocked.CompareExchange(ref _throwOnNextStartupForTests, 0, 1); + Interlocked.CompareExchange(ref _throwOnNextStartupForTests, FALSE, TRUE); } } @@ -77,7 +86,7 @@ public static SampleProfilerSession StartNew(IDiagnosticLogger? logger = null) { var client = new DiagnosticsClient(Environment.ProcessId); - if (Interlocked.CompareExchange(ref _throwOnNextStartupForTests, 0, 1) == 1) + if (Interlocked.CompareExchange(ref _throwOnNextStartupForTests, FALSE, TRUE) == TRUE) { throw new Exception("Test exception"); } diff --git a/src/Sentry.Profiling/SamplingTransactionProfilerFactory.cs b/src/Sentry.Profiling/SamplingTransactionProfilerFactory.cs index 5f0d54453d..84bc92cf0a 100644 --- a/src/Sentry.Profiling/SamplingTransactionProfilerFactory.cs +++ b/src/Sentry.Profiling/SamplingTransactionProfilerFactory.cs @@ -6,14 +6,21 @@ namespace Sentry.Profiling; internal class SamplingTransactionProfilerFactory : IDisposable, ITransactionProfilerFactory { // We only allow a single profile so let's keep track of the current status. +#if NET9_0_OR_GREATER + internal bool _inProgress = FALSE; + + const bool TRUE = true; + const bool FALSE = false; +#else internal int _inProgress = FALSE; + const int TRUE = 1; + const int FALSE = 0; +#endif + // Whether the session startup took longer than the given timeout. internal bool StartupTimedOut { get; } - private const int TRUE = 1; - private const int FALSE = 0; - // Stop profiling after the given number of milliseconds. private const int TIME_LIMIT_MS = 30_000; diff --git a/src/Sentry/Ben.BlockingDetector/DetectBlockingSynchronizationContext.cs b/src/Sentry/Ben.BlockingDetector/DetectBlockingSynchronizationContext.cs index 2e4aeb08ca..701465b5d3 100644 --- a/src/Sentry/Ben.BlockingDetector/DetectBlockingSynchronizationContext.cs +++ b/src/Sentry/Ben.BlockingDetector/DetectBlockingSynchronizationContext.cs @@ -10,8 +10,8 @@ internal sealed class DetectBlockingSynchronizationContext : SynchronizationCont internal int _isSuppressed; - internal void Suppress() => Interlocked.Exchange(ref _isSuppressed, _isSuppressed + 1); - internal void Restore() => Interlocked.Exchange(ref _isSuppressed, _isSuppressed - 1); + internal void Suppress() => Interlocked.Increment(ref _isSuppressed); + internal void Restore() => Interlocked.Decrement(ref _isSuppressed); public DetectBlockingSynchronizationContext(IBlockingMonitor monitor) { diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs index 5170a64233..05f98872d1 100644 --- a/src/Sentry/Internal/Hub.cs +++ b/src/Sentry/Internal/Hub.cs @@ -24,15 +24,30 @@ internal class Hub : IHub, IDisposable private readonly MemoryMonitor? _memoryMonitor; #endif +#if NET9_0_OR_GREATER + private bool _isPersistedSessionRecovered; + + const bool TRUE = true; + const bool FALSE = false; +#else private int _isPersistedSessionRecovered; + const int TRUE = 1; + const int FALSE = 0; +#endif + // Internal for testability internal ConditionalWeakTable ExceptionToSpanMap { get; } = new(); internal IInternalScopeManager ScopeManager { get; } - private int _isEnabled = 1; - public bool IsEnabled => _isEnabled == 1; +#if NET9_0_OR_GREATER + private bool _isEnabled = TRUE; +#else + private int _isEnabled = TRUE; +#endif + + public bool IsEnabled => _isEnabled != FALSE; internal SentryOptions Options => _options; @@ -356,7 +371,7 @@ public TransactionContext ContinueTrace( public void StartSession() { // Attempt to recover persisted session left over from previous run - if (Interlocked.Exchange(ref _isPersistedSessionRecovered, 1) != 1) + if (Interlocked.Exchange(ref _isPersistedSessionRecovered, TRUE) != TRUE) { try { @@ -835,7 +850,7 @@ public void Dispose() { _options.LogInfo("Disposing the Hub."); - if (Interlocked.Exchange(ref _isEnabled, 0) != 1) + if (Interlocked.Exchange(ref _isEnabled, FALSE) != TRUE) { return; } diff --git a/src/Sentry/Threading/ScopedCountdownLock.cs b/src/Sentry/Threading/ScopedCountdownLock.cs index f3d992f893..68bcd3dcc5 100644 --- a/src/Sentry/Threading/ScopedCountdownLock.cs +++ b/src/Sentry/Threading/ScopedCountdownLock.cs @@ -13,12 +13,23 @@ namespace Sentry.Threading; internal sealed class ScopedCountdownLock : IDisposable { private readonly CountdownEvent _event; + +#if NET9_0_OR_GREATER + private volatile bool _isEngaged; + + const bool TRUE = true; + const bool FALSE = false; +#else private volatile int _isEngaged; + const int TRUE = 1; + const int FALSE = 0; +#endif + internal ScopedCountdownLock() { _event = new CountdownEvent(1); - _isEngaged = 0; + _isEngaged = FALSE; } /// @@ -31,13 +42,13 @@ internal ScopedCountdownLock() /// Gets the number of remaining required to exit in order to set/signal the event while a is active. /// When and while a is active, no more can be entered. /// - internal int Count => _isEngaged == 1 ? _event.CurrentCount : _event.CurrentCount - 1; + internal int Count => _isEngaged == TRUE ? _event.CurrentCount : _event.CurrentCount - 1; /// /// Returns when a is active and the event can be set/signaled by reaching . /// Returns when the can only reach the initial count of when no is active any longer. /// - internal bool IsEngaged => _isEngaged == 1; + internal bool IsEngaged => _isEngaged == TRUE; /// /// No will be entered when the has reached , or while the lock is engaged via an active . @@ -79,7 +90,7 @@ private void ExitCounterScope() /// internal LockScope TryEnterLockScope() { - if (Interlocked.CompareExchange(ref _isEngaged, 1, 0) == 0) + if (Interlocked.CompareExchange(ref _isEngaged, TRUE, FALSE) == FALSE) { Debug.Assert(_event.CurrentCount >= 1); _ = _event.Signal(); // decrement the initial count of 1, so that the event can be set with the count reaching 0 when all entered 'CounterScope' instances have exited @@ -94,7 +105,7 @@ private void ExitLockScope() Debug.Assert(_event.IsSet); _event.Reset(); // reset the signaled event to the initial count of 1, so that new 'CounterScope' instances can be entered again - if (Interlocked.CompareExchange(ref _isEngaged, 0, 1) != 1) + if (Interlocked.CompareExchange(ref _isEngaged, FALSE, TRUE) != TRUE) { Debug.Fail("The Lock should have not been disengaged without being engaged first."); } diff --git a/src/Sentry/TransactionTracer.cs b/src/Sentry/TransactionTracer.cs index 767ccf977c..f7b77ed7ae 100644 --- a/src/Sentry/TransactionTracer.cs +++ b/src/Sentry/TransactionTracer.cs @@ -12,9 +12,20 @@ public class TransactionTracer : IBaseTracer, ITransactionTracer private readonly IHub _hub; private readonly SentryOptions? _options; private readonly Timer? _idleTimer; - private long _cancelIdleTimeout; private readonly SentryStopwatch _stopwatch = SentryStopwatch.StartNew(); +#if NET9_0_OR_GREATER + private bool _cancelIdleTimeout; + + const bool TRUE = true; + const bool FALSE = false; +#else + private int _cancelIdleTimeout; + + const int TRUE = 1; + const int FALSE = 0; +#endif + private readonly Instrumenter _instrumenter = Instrumenter.Sentry; bool IBaseTracer.IsOtelInstrumenter => _instrumenter == Instrumenter.OpenTelemetry; @@ -247,7 +258,7 @@ internal TransactionTracer(IHub hub, ITransactionContext context, TimeSpan? idle // Set idle timer only if an idle timeout has been provided directly if (idleTimeout.HasValue) { - _cancelIdleTimeout = 1; // Timer will be cancelled once, atomically setting this back to 0 + _cancelIdleTimeout = TRUE; // Timer will be cancelled once, atomically setting this back to false _idleTimer = new Timer(state => { if (state is not TransactionTracer transactionTracer) @@ -362,7 +373,7 @@ public void Clear() public void Finish() { _options?.LogDebug("Attempting to finish Transaction {0}.", SpanId); - if (Interlocked.Exchange(ref _cancelIdleTimeout, 0) == 1) + if (Interlocked.Exchange(ref _cancelIdleTimeout, FALSE) == TRUE) { _options?.LogDebug("Disposing of idle timer for Transaction {0}.", SpanId); _idleTimer?.Change(Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); From f0d95a3e51eb886fc7ad794f6ba520ff72f5c1be Mon Sep 17 00:00:00 2001 From: Jonathan Gilbert Date: Wed, 1 Oct 2025 19:50:45 -0500 Subject: [PATCH 02/10] Added changes to CHANGELOG.md. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fbe102adb8..0ae8965520 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Use new `Interlocked.Exchange`/`CompareExchange` overloads that support `bool` values ([#4585](https://github.com/getsentry/sentry-dotnet/pull/4585)) + ### Dependencies - Bump Java SDK from v8.22.0 to v8.23.0 ([#4586](https://github.com/getsentry/sentry-dotnet/pull/4586)) From f1c40d0b2eedca8d7bac379309be1becbc6553a3 Mon Sep 17 00:00:00 2001 From: Jonathan Gilbert Date: Mon, 6 Oct 2025 20:25:00 -0500 Subject: [PATCH 03/10] Revert "Added changes to CHANGELOG.md." This reverts commit f0d95a3e51eb886fc7ad794f6ba520ff72f5c1be. Per discussion, this is not a user-facing change. --- CHANGELOG.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ae8965520..fbe102adb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,6 @@ ## Unreleased -- Use new `Interlocked.Exchange`/`CompareExchange` overloads that support `bool` values ([#4585](https://github.com/getsentry/sentry-dotnet/pull/4585)) - ### Dependencies - Bump Java SDK from v8.22.0 to v8.23.0 ([#4586](https://github.com/getsentry/sentry-dotnet/pull/4586)) From 5f83895c927628cfdf377287480ce70843a2b8a3 Mon Sep 17 00:00:00 2001 From: Jonathan Gilbert Date: Mon, 6 Oct 2025 11:05:33 -0500 Subject: [PATCH 04/10] Centralized interlocked functionality in new class InterlockedBoolean. --- .../SentryDatabaseLogging.cs | 14 +----- src/Sentry.Profiling/SampleProfilerSession.cs | 20 ++------ .../SamplingTransactionProfilerFactory.cs | 22 +++------ src/Sentry/Internal/Hub.cs | 24 ++-------- src/Sentry/Internal/InterlockedBoolean.cs | 46 +++++++++++++++++++ src/Sentry/Threading/ScopedCountdownLock.cs | 24 ++++------ src/Sentry/TransactionTracer.cs | 16 ++----- 7 files changed, 75 insertions(+), 91 deletions(-) create mode 100644 src/Sentry/Internal/InterlockedBoolean.cs diff --git a/src/Sentry.EntityFramework/SentryDatabaseLogging.cs b/src/Sentry.EntityFramework/SentryDatabaseLogging.cs index d7855fa1cb..a43a29f907 100644 --- a/src/Sentry.EntityFramework/SentryDatabaseLogging.cs +++ b/src/Sentry.EntityFramework/SentryDatabaseLogging.cs @@ -5,24 +5,14 @@ namespace Sentry.EntityFramework; /// internal static class SentryDatabaseLogging { -#if NET9_0_OR_GREATER - private static bool _init; - - const bool TRUE = true; - const bool FALSE = false; -#else - private static int _init; - - const int TRUE = 1; - const int FALSE = 0; -#endif + private static InterlockedBoolean _init; internal static SentryCommandInterceptor? UseBreadcrumbs( IQueryLogger? queryLogger = null, bool initOnce = true, IDiagnosticLogger? diagnosticLogger = null) { - if (initOnce && Interlocked.Exchange(ref _init, TRUE) != FALSE) + if (initOnce && _init.Exchange(true)) { diagnosticLogger?.LogWarning("{0}.{1} was already executed.", nameof(SentryDatabaseLogging), nameof(UseBreadcrumbs)); diff --git a/src/Sentry.Profiling/SampleProfilerSession.cs b/src/Sentry.Profiling/SampleProfilerSession.cs index ee7562cb46..91eba73fe2 100644 --- a/src/Sentry.Profiling/SampleProfilerSession.cs +++ b/src/Sentry.Profiling/SampleProfilerSession.cs @@ -56,27 +56,17 @@ private SampleProfilerSession(SentryStopwatch stopwatch, EventPipeSession sessio public TraceLog TraceLog => EventSource.TraceLog; -#if NET9_0_OR_GREATER - private static bool _throwOnNextStartupForTests = FALSE; - - const bool TRUE = true; - const bool FALSE = false; -#else - private static int _throwOnNextStartupForTests = FALSE; - - const int TRUE = 1; - const int FALSE = 0; -#endif + private static InterlockedBoolean _throwOnNextStartupForTests = false; internal static bool ThrowOnNextStartupForTests { - get { return _throwOnNextStartupForTests != FALSE; } + get { return _throwOnNextStartupForTests; } set { if (value) - Interlocked.CompareExchange(ref _throwOnNextStartupForTests, TRUE, FALSE); + _throwOnNextStartupForTests.CompareExchange(true, false); else - Interlocked.CompareExchange(ref _throwOnNextStartupForTests, FALSE, TRUE); + _throwOnNextStartupForTests.CompareExchange(false, true); } } @@ -86,7 +76,7 @@ public static SampleProfilerSession StartNew(IDiagnosticLogger? logger = null) { var client = new DiagnosticsClient(Environment.ProcessId); - if (Interlocked.CompareExchange(ref _throwOnNextStartupForTests, FALSE, TRUE) == TRUE) + if (_throwOnNextStartupForTests.CompareExchange(false, true) == true) { throw new Exception("Test exception"); } diff --git a/src/Sentry.Profiling/SamplingTransactionProfilerFactory.cs b/src/Sentry.Profiling/SamplingTransactionProfilerFactory.cs index 84bc92cf0a..1d3d03c5f3 100644 --- a/src/Sentry.Profiling/SamplingTransactionProfilerFactory.cs +++ b/src/Sentry.Profiling/SamplingTransactionProfilerFactory.cs @@ -6,17 +6,7 @@ namespace Sentry.Profiling; internal class SamplingTransactionProfilerFactory : IDisposable, ITransactionProfilerFactory { // We only allow a single profile so let's keep track of the current status. -#if NET9_0_OR_GREATER - internal bool _inProgress = FALSE; - - const bool TRUE = true; - const bool FALSE = false; -#else - internal int _inProgress = FALSE; - - const int TRUE = 1; - const int FALSE = 0; -#endif + internal InterlockedBoolean _inProgress = false; // Whether the session startup took longer than the given timeout. internal bool StartupTimedOut { get; } @@ -57,12 +47,12 @@ public SamplingTransactionProfilerFactory(SentryOptions options, TimeSpan startu public ITransactionProfiler? Start(ITransactionTracer _, CancellationToken cancellationToken) { // Start a profiler if one wasn't running yet. - if (!_errorLogged && Interlocked.Exchange(ref _inProgress, TRUE) == FALSE) + if (!_errorLogged && !_inProgress.Exchange(true)) { if (!_sessionTask.IsCompleted) { _options.LogWarning("Cannot start a sampling profiler, the session hasn't started yet."); - _inProgress = FALSE; + _inProgress = false; return null; } @@ -70,7 +60,7 @@ public SamplingTransactionProfilerFactory(SentryOptions options, TimeSpan startu { _options.LogWarning("Cannot start a sampling profiler because the session startup has failed. This is a permanent error and no future transactions will be sampled."); _errorLogged = true; - _inProgress = FALSE; + _inProgress = false; return null; } @@ -79,13 +69,13 @@ public SamplingTransactionProfilerFactory(SentryOptions options, TimeSpan startu { return new SamplingTransactionProfiler(_options, _sessionTask.Result, TIME_LIMIT_MS, cancellationToken) { - OnFinish = () => _inProgress = FALSE + OnFinish = () => _inProgress = false }; } catch (Exception e) { _options.LogError(e, "Failed to start a profiler session."); - _inProgress = FALSE; + _inProgress = false; } } return null; diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs index 05f98872d1..5b0a71bd7f 100644 --- a/src/Sentry/Internal/Hub.cs +++ b/src/Sentry/Internal/Hub.cs @@ -24,30 +24,16 @@ internal class Hub : IHub, IDisposable private readonly MemoryMonitor? _memoryMonitor; #endif -#if NET9_0_OR_GREATER - private bool _isPersistedSessionRecovered; - - const bool TRUE = true; - const bool FALSE = false; -#else - private int _isPersistedSessionRecovered; - - const int TRUE = 1; - const int FALSE = 0; -#endif + private InterlockedBoolean _isPersistedSessionRecovered; // Internal for testability internal ConditionalWeakTable ExceptionToSpanMap { get; } = new(); internal IInternalScopeManager ScopeManager { get; } -#if NET9_0_OR_GREATER - private bool _isEnabled = TRUE; -#else - private int _isEnabled = TRUE; -#endif + private InterlockedBoolean _isEnabled = true; - public bool IsEnabled => _isEnabled != FALSE; + public bool IsEnabled => _isEnabled; internal SentryOptions Options => _options; @@ -371,7 +357,7 @@ public TransactionContext ContinueTrace( public void StartSession() { // Attempt to recover persisted session left over from previous run - if (Interlocked.Exchange(ref _isPersistedSessionRecovered, TRUE) != TRUE) + if (_isPersistedSessionRecovered.Exchange(true) != true) { try { @@ -850,7 +836,7 @@ public void Dispose() { _options.LogInfo("Disposing the Hub."); - if (Interlocked.Exchange(ref _isEnabled, FALSE) != TRUE) + if (!_isEnabled.Exchange(false)) { return; } diff --git a/src/Sentry/Internal/InterlockedBoolean.cs b/src/Sentry/Internal/InterlockedBoolean.cs new file mode 100644 index 0000000000..56c3e750d7 --- /dev/null +++ b/src/Sentry/Internal/InterlockedBoolean.cs @@ -0,0 +1,46 @@ +namespace Sentry.Internal; + +#if NET9_0_OR_GREATER +using TBool = bool; +#else +using TBool = int; +#endif + +internal struct InterlockedBoolean +{ + private volatile TBool _value; + +#if NET9_0_OR_GREATER + private const TBool True = true; + private const TBool False = false; +#else + private const TBool True = 1; + private const TBool False = 0; +#endif + + public InterlockedBoolean() { } + + public InterlockedBoolean(bool value) { _value = value ? True : False; } + + public static implicit operator bool(InterlockedBoolean? _this) => (_this != null) && (_this.Value._value != False); + public static implicit operator InterlockedBoolean(bool _this) => new InterlockedBoolean(_this); + + public bool Exchange(bool newValue) + { + TBool localNewValue = newValue ? True : False; + + TBool localReturnValue = Interlocked.Exchange(ref _value, localNewValue); + + return (localReturnValue != False); + } + + public bool CompareExchange(bool value, bool comparand) + { + TBool localValue = value ? True : False; + TBool localComparand = comparand ? True : False; + + TBool localReturnValue = Interlocked.CompareExchange(ref _value, localValue, localComparand); + + return (localReturnValue != False); + } +} diff --git a/src/Sentry/Threading/ScopedCountdownLock.cs b/src/Sentry/Threading/ScopedCountdownLock.cs index 68bcd3dcc5..e321b8e705 100644 --- a/src/Sentry/Threading/ScopedCountdownLock.cs +++ b/src/Sentry/Threading/ScopedCountdownLock.cs @@ -1,5 +1,7 @@ namespace Sentry.Threading; +using Sentry.Internal; + /// /// A synchronization primitive that tracks the amount of s held, /// and is signaled when the count reaches zero while a is held. @@ -14,22 +16,12 @@ internal sealed class ScopedCountdownLock : IDisposable { private readonly CountdownEvent _event; -#if NET9_0_OR_GREATER - private volatile bool _isEngaged; - - const bool TRUE = true; - const bool FALSE = false; -#else - private volatile int _isEngaged; - - const int TRUE = 1; - const int FALSE = 0; -#endif + private InterlockedBoolean _isEngaged; internal ScopedCountdownLock() { _event = new CountdownEvent(1); - _isEngaged = FALSE; + _isEngaged = false; } /// @@ -42,13 +34,13 @@ internal ScopedCountdownLock() /// Gets the number of remaining required to exit in order to set/signal the event while a is active. /// When and while a is active, no more can be entered. /// - internal int Count => _isEngaged == TRUE ? _event.CurrentCount : _event.CurrentCount - 1; + internal int Count => _isEngaged ? _event.CurrentCount : _event.CurrentCount - 1; /// /// Returns when a is active and the event can be set/signaled by reaching . /// Returns when the can only reach the initial count of when no is active any longer. /// - internal bool IsEngaged => _isEngaged == TRUE; + internal bool IsEngaged => _isEngaged; /// /// No will be entered when the has reached , or while the lock is engaged via an active . @@ -90,7 +82,7 @@ private void ExitCounterScope() /// internal LockScope TryEnterLockScope() { - if (Interlocked.CompareExchange(ref _isEngaged, TRUE, FALSE) == FALSE) + if (_isEngaged.CompareExchange(true, false) == false) { Debug.Assert(_event.CurrentCount >= 1); _ = _event.Signal(); // decrement the initial count of 1, so that the event can be set with the count reaching 0 when all entered 'CounterScope' instances have exited @@ -105,7 +97,7 @@ private void ExitLockScope() Debug.Assert(_event.IsSet); _event.Reset(); // reset the signaled event to the initial count of 1, so that new 'CounterScope' instances can be entered again - if (Interlocked.CompareExchange(ref _isEngaged, FALSE, TRUE) != TRUE) + if (_isEngaged.CompareExchange(false, true) != true) { Debug.Fail("The Lock should have not been disengaged without being engaged first."); } diff --git a/src/Sentry/TransactionTracer.cs b/src/Sentry/TransactionTracer.cs index f7b77ed7ae..1f965deef6 100644 --- a/src/Sentry/TransactionTracer.cs +++ b/src/Sentry/TransactionTracer.cs @@ -14,17 +14,7 @@ public class TransactionTracer : IBaseTracer, ITransactionTracer private readonly Timer? _idleTimer; private readonly SentryStopwatch _stopwatch = SentryStopwatch.StartNew(); -#if NET9_0_OR_GREATER - private bool _cancelIdleTimeout; - - const bool TRUE = true; - const bool FALSE = false; -#else - private int _cancelIdleTimeout; - - const int TRUE = 1; - const int FALSE = 0; -#endif + private InterlockedBoolean _cancelIdleTimeout; private readonly Instrumenter _instrumenter = Instrumenter.Sentry; @@ -258,7 +248,7 @@ internal TransactionTracer(IHub hub, ITransactionContext context, TimeSpan? idle // Set idle timer only if an idle timeout has been provided directly if (idleTimeout.HasValue) { - _cancelIdleTimeout = TRUE; // Timer will be cancelled once, atomically setting this back to false + _cancelIdleTimeout = true; // Timer will be cancelled once, atomically setting this back to false _idleTimer = new Timer(state => { if (state is not TransactionTracer transactionTracer) @@ -373,7 +363,7 @@ public void Clear() public void Finish() { _options?.LogDebug("Attempting to finish Transaction {0}.", SpanId); - if (Interlocked.Exchange(ref _cancelIdleTimeout, FALSE) == TRUE) + if (_cancelIdleTimeout.Exchange(false) == true) { _options?.LogDebug("Disposing of idle timer for Transaction {0}.", SpanId); _idleTimer?.Change(Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); From 344c66eb980e7ee6f8ce1aef5ec33ee91cb0c992 Mon Sep 17 00:00:00 2001 From: Jonathan Gilbert Date: Mon, 6 Oct 2025 23:05:06 -0500 Subject: [PATCH 05/10] Made the _value field in InterlockedBoolean internal to allow for testing. Added InterlockedBooleanTests.cs to Sentry.Tests. Added a missing using statement to SentryDatabaseLogging.cs. --- .../SentryDatabaseLogging.cs | 2 + src/Sentry/Internal/InterlockedBoolean.cs | 2 +- .../Internals/InterlockedBooleanTests.cs | 149 ++++++++++++++++++ 3 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 test/Sentry.Tests/Internals/InterlockedBooleanTests.cs diff --git a/src/Sentry.EntityFramework/SentryDatabaseLogging.cs b/src/Sentry.EntityFramework/SentryDatabaseLogging.cs index a43a29f907..5f208593ef 100644 --- a/src/Sentry.EntityFramework/SentryDatabaseLogging.cs +++ b/src/Sentry.EntityFramework/SentryDatabaseLogging.cs @@ -1,5 +1,7 @@ namespace Sentry.EntityFramework; +using Sentry.Internal; + /// /// Sentry Database Logger /// diff --git a/src/Sentry/Internal/InterlockedBoolean.cs b/src/Sentry/Internal/InterlockedBoolean.cs index 56c3e750d7..05654e7cc8 100644 --- a/src/Sentry/Internal/InterlockedBoolean.cs +++ b/src/Sentry/Internal/InterlockedBoolean.cs @@ -8,7 +8,7 @@ namespace Sentry.Internal; internal struct InterlockedBoolean { - private volatile TBool _value; + internal volatile TBool _value; #if NET9_0_OR_GREATER private const TBool True = true; diff --git a/test/Sentry.Tests/Internals/InterlockedBooleanTests.cs b/test/Sentry.Tests/Internals/InterlockedBooleanTests.cs new file mode 100644 index 0000000000..7acd4ba9c0 --- /dev/null +++ b/test/Sentry.Tests/Internals/InterlockedBooleanTests.cs @@ -0,0 +1,149 @@ +namespace Sentry.Tests.Internals; + +#if NET9_0_OR_GREATER +using TBool = bool; +#else +using TBool = int; +#endif + +public class InterlockedBooleanTests +{ +#if NET9_0_OR_GREATER + private const TBool True = true; + private const TBool False = false; +#else + private const TBool True = 1; + private const TBool False = 0; +#endif + + private TBool ToTBool(bool value) => value ? True : False; + private bool FromTBool(TBool value) => (value != False); + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void InterlockedBoolean_Constructor_ConstructsExpected(bool value) + { + // Arrange + var tboolValue = ToTBool(value); + + // Act + var actual = new InterlockedBoolean(value); + + // Assert + actual._value.Should().Be(tboolValue); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void InterlockedBoolean_ImplicitToBool_ReturnsExpected(bool value) + { + // Arrange + var sut = new InterlockedBoolean(value); + + // Act + bool actual = sut; + + // Assert + actual.Should().Be(value); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void InterlockedBoolean_ImplicitFromBool_ReturnsExpected(bool value) + { + // Arrange + var tboolValue = ToTBool(value); + + // Act + InterlockedBoolean actual = value; + + // Assert + actual._value.Should().Be(tboolValue); + } + + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void InterlockedBoolean_Exchange_ReturnsExpected(bool initialState, bool newValue) + { + // Arrange + var sut = new InterlockedBoolean(initialState); + + // Act + var result = sut.Exchange(newValue); + + // Assert + result.Should().Be(initialState); + } + + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public void InterlockedBoolean_Exchange_SetsExpectedNewState(bool initialState, bool newValue) + { + // Arrange + var sut = new InterlockedBoolean(initialState); + + var tboolNewValue = ToTBool(newValue); + + // Act + var result = sut.Exchange(newValue); + + // Assert + sut._value.Should().Be(tboolNewValue); + } + + [Theory] + [InlineData(false, false, false)] + [InlineData(false, false, true)] + [InlineData(false, true, false)] + [InlineData(false, true, true)] + [InlineData(true, false, false)] + [InlineData(true, false, true)] + [InlineData(true, true, false)] + [InlineData(true, true, true)] + public void InterlockedBoolean_CompareExchange_ReturnsExpected(bool initialState, bool comparand, bool newValue) + { + // Arrange + var sut = new InterlockedBoolean(initialState); + + // Act + var result = sut.CompareExchange(comparand, newValue); + + // Assert + result.Should().Be(initialState); + } + + [Theory] + [InlineData(false, false, false)] + [InlineData(false, false, true)] + [InlineData(false, true, false)] + [InlineData(false, true, true)] + [InlineData(true, false, false)] + [InlineData(true, false, true)] + [InlineData(true, true, false)] + [InlineData(true, true, true)] + public void InterlockedBoolean_CompareExchange_SetsExpectedNewState(bool initialState, bool comparand, bool newValue) + { + // Arrange + var sut = new InterlockedBoolean(initialState); + + var tboolExpected = ToTBool( + initialState == comparand + ? newValue + : initialState); + + // Act + sut.CompareExchange(newValue, comparand); + + // Assert + sut._value.Should().Be(tboolExpected); + } +} From 01414209519a11a409da664369dbefb69f3e049f Mon Sep 17 00:00:00 2001 From: Jonathan Gilbert Date: Mon, 6 Oct 2025 23:09:36 -0500 Subject: [PATCH 06/10] Fixed parameter order in call to InterlockedBoolean.CompareExchange in test theory InterlockedBoolean_CompareExchange_ReturnsExpected. --- test/Sentry.Tests/Internals/InterlockedBooleanTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Sentry.Tests/Internals/InterlockedBooleanTests.cs b/test/Sentry.Tests/Internals/InterlockedBooleanTests.cs index 7acd4ba9c0..b9c9e71770 100644 --- a/test/Sentry.Tests/Internals/InterlockedBooleanTests.cs +++ b/test/Sentry.Tests/Internals/InterlockedBooleanTests.cs @@ -115,7 +115,7 @@ public void InterlockedBoolean_CompareExchange_ReturnsExpected(bool initialState var sut = new InterlockedBoolean(initialState); // Act - var result = sut.CompareExchange(comparand, newValue); + var result = sut.CompareExchange(newValue, comparand); // Assert result.Should().Be(initialState); From ab4c755a48e324ca6ab34e2590f454c820adb4be Mon Sep 17 00:00:00 2001 From: Jonathan Gilbert Date: Wed, 8 Oct 2025 12:23:56 -0500 Subject: [PATCH 07/10] PR feedback. - Moved using statements outside of namespaces in SentryDatabaseLogging.cs and SentryCountdownLock.cs. Specified preference for this in .editorconfig. - Renamed _init back to Init in SentryDatabaseLogging.cs. - Moved type aliases in InterlockedBoolean.cs and InterlockedBooleanTests.cs outside of the namespace and changed them from built-in type names to explicit references to the underlying types. - Made _value in InterlockedBoolean private and added an internal test-specific property instead. - Added MethodImpl hints to the methods it InterlockedBoolean.cs requesting aggressive inlining. - Changed parameters to the implicit conversion operatons in InterlockedBoolean from _this to @this. - Standardized .Should.Be(..) assertions in InterlockedBooleanTests.cs so that the expectation is always named expected. --- .../.editorconfig | 4 ++- .../SentryDatabaseLogging.cs | 8 ++--- src/Sentry/Internal/InterlockedBoolean.cs | 22 +++++++++---- src/Sentry/Threading/ScopedCountdownLock.cs | 4 +-- .../Internals/InterlockedBooleanTests.cs | 33 ++++++++++--------- 5 files changed, 42 insertions(+), 29 deletions(-) diff --git a/src/Sentry.Azure.Functions.Worker/.editorconfig b/src/Sentry.Azure.Functions.Worker/.editorconfig index bdf468a081..75b15d3f30 100644 --- a/src/Sentry.Azure.Functions.Worker/.editorconfig +++ b/src/Sentry.Azure.Functions.Worker/.editorconfig @@ -2,4 +2,6 @@ # Reason: Azure Functions worker doesn't set SynchronizationContext but Durable Functions does and required affinity. # (https://github.com/Azure/azure-functions-dotnet-worker/issues/1520) -dotnet_diagnostic.CA2007.severity = none \ No newline at end of file +dotnet_diagnostic.CA2007.severity = none + +dotnet_style_namespace_declaration_with_usings_placement = prefer_outside_namespace diff --git a/src/Sentry.EntityFramework/SentryDatabaseLogging.cs b/src/Sentry.EntityFramework/SentryDatabaseLogging.cs index 5f208593ef..174af13873 100644 --- a/src/Sentry.EntityFramework/SentryDatabaseLogging.cs +++ b/src/Sentry.EntityFramework/SentryDatabaseLogging.cs @@ -1,20 +1,20 @@ -namespace Sentry.EntityFramework; - using Sentry.Internal; +namespace Sentry.EntityFramework; + /// /// Sentry Database Logger /// internal static class SentryDatabaseLogging { - private static InterlockedBoolean _init; + private static InterlockedBoolean Init; internal static SentryCommandInterceptor? UseBreadcrumbs( IQueryLogger? queryLogger = null, bool initOnce = true, IDiagnosticLogger? diagnosticLogger = null) { - if (initOnce && _init.Exchange(true)) + if (initOnce && Init.Exchange(true)) { diagnosticLogger?.LogWarning("{0}.{1} was already executed.", nameof(SentryDatabaseLogging), nameof(UseBreadcrumbs)); diff --git a/src/Sentry/Internal/InterlockedBoolean.cs b/src/Sentry/Internal/InterlockedBoolean.cs index 05654e7cc8..61e7f0968d 100644 --- a/src/Sentry/Internal/InterlockedBoolean.cs +++ b/src/Sentry/Internal/InterlockedBoolean.cs @@ -1,14 +1,17 @@ -namespace Sentry.Internal; - #if NET9_0_OR_GREATER -using TBool = bool; +using TBool = System.Boolean; #else -using TBool = int; +using TBool = System.Int32; #endif +namespace Sentry.Internal; + internal struct InterlockedBoolean { - internal volatile TBool _value; + private volatile TBool _value; + + [Browsable(false)] + internal TBool ValueForTests => _value; #if NET9_0_OR_GREATER private const TBool True = true; @@ -20,11 +23,15 @@ internal struct InterlockedBoolean public InterlockedBoolean() { } + [MethodImpl(MethodImplOptions.AggressiveInlining)] public InterlockedBoolean(bool value) { _value = value ? True : False; } - public static implicit operator bool(InterlockedBoolean? _this) => (_this != null) && (_this.Value._value != False); - public static implicit operator InterlockedBoolean(bool _this) => new InterlockedBoolean(_this); + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static implicit operator bool(InterlockedBoolean @this) => (@this._value != False); + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static implicit operator InterlockedBoolean(bool @this) => new InterlockedBoolean(@this); + [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool Exchange(bool newValue) { TBool localNewValue = newValue ? True : False; @@ -34,6 +41,7 @@ public bool Exchange(bool newValue) return (localReturnValue != False); } + [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool CompareExchange(bool value, bool comparand) { TBool localValue = value ? True : False; diff --git a/src/Sentry/Threading/ScopedCountdownLock.cs b/src/Sentry/Threading/ScopedCountdownLock.cs index e321b8e705..1cf38678fb 100644 --- a/src/Sentry/Threading/ScopedCountdownLock.cs +++ b/src/Sentry/Threading/ScopedCountdownLock.cs @@ -1,7 +1,7 @@ -namespace Sentry.Threading; - using Sentry.Internal; +namespace Sentry.Threading; + /// /// A synchronization primitive that tracks the amount of s held, /// and is signaled when the count reaches zero while a is held. diff --git a/test/Sentry.Tests/Internals/InterlockedBooleanTests.cs b/test/Sentry.Tests/Internals/InterlockedBooleanTests.cs index b9c9e71770..ff2b1cf50e 100644 --- a/test/Sentry.Tests/Internals/InterlockedBooleanTests.cs +++ b/test/Sentry.Tests/Internals/InterlockedBooleanTests.cs @@ -1,11 +1,11 @@ -namespace Sentry.Tests.Internals; - #if NET9_0_OR_GREATER -using TBool = bool; +using TBool = System.Boolean; #else -using TBool = int; +using TBool = System.Int32; #endif +namespace Sentry.Tests.Internals; + public class InterlockedBooleanTests { #if NET9_0_OR_GREATER @@ -25,13 +25,13 @@ public class InterlockedBooleanTests public void InterlockedBoolean_Constructor_ConstructsExpected(bool value) { // Arrange - var tboolValue = ToTBool(value); + var expected = ToTBool(value); // Act var actual = new InterlockedBoolean(value); // Assert - actual._value.Should().Be(tboolValue); + actual.ValueForTests.Should().Be(expected); } [Theory] @@ -41,12 +41,13 @@ public void InterlockedBoolean_ImplicitToBool_ReturnsExpected(bool value) { // Arrange var sut = new InterlockedBoolean(value); + var expected = value; // Act bool actual = sut; // Assert - actual.Should().Be(value); + actual.Should().Be(expected); } [Theory] @@ -55,13 +56,13 @@ public void InterlockedBoolean_ImplicitToBool_ReturnsExpected(bool value) public void InterlockedBoolean_ImplicitFromBool_ReturnsExpected(bool value) { // Arrange - var tboolValue = ToTBool(value); + var expected = ToTBool(value); // Act InterlockedBoolean actual = value; // Assert - actual._value.Should().Be(tboolValue); + actual.ValueForTests.Should().Be(expected); } [Theory] @@ -73,12 +74,13 @@ public void InterlockedBoolean_Exchange_ReturnsExpected(bool initialState, bool { // Arrange var sut = new InterlockedBoolean(initialState); + var expected = initialState; // Act var result = sut.Exchange(newValue); // Assert - result.Should().Be(initialState); + result.Should().Be(expected); } [Theory] @@ -91,13 +93,13 @@ public void InterlockedBoolean_Exchange_SetsExpectedNewState(bool initialState, // Arrange var sut = new InterlockedBoolean(initialState); - var tboolNewValue = ToTBool(newValue); + var expected = ToTBool(newValue); // Act var result = sut.Exchange(newValue); // Assert - sut._value.Should().Be(tboolNewValue); + sut.ValueForTests.Should().Be(expected); } [Theory] @@ -113,12 +115,13 @@ public void InterlockedBoolean_CompareExchange_ReturnsExpected(bool initialState { // Arrange var sut = new InterlockedBoolean(initialState); + var expected = initialState; // Act var result = sut.CompareExchange(newValue, comparand); // Assert - result.Should().Be(initialState); + result.Should().Be(expected); } [Theory] @@ -135,7 +138,7 @@ public void InterlockedBoolean_CompareExchange_SetsExpectedNewState(bool initial // Arrange var sut = new InterlockedBoolean(initialState); - var tboolExpected = ToTBool( + var expected = ToTBool( initialState == comparand ? newValue : initialState); @@ -144,6 +147,6 @@ public void InterlockedBoolean_CompareExchange_SetsExpectedNewState(bool initial sut.CompareExchange(newValue, comparand); // Assert - sut._value.Should().Be(tboolExpected); + sut.ValueForTests.Should().Be(expected); } } From a446ec98dbb51fceb16c58b947f291a5092ca4ee Mon Sep 17 00:00:00 2001 From: Jonathan Gilbert Date: Wed, 8 Oct 2025 12:35:22 -0500 Subject: [PATCH 08/10] Removed unnecessary CompareExchange calls in the ThrowOnNextStartupForTests property setter in SampleProfilerSession.cs. One straggler from the PR review feedback that I somehow missed: explicitly discard an unused return value in one of the tests. --- src/Sentry.Profiling/SampleProfilerSession.cs | 8 +------- test/Sentry.Tests/Internals/InterlockedBooleanTests.cs | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Sentry.Profiling/SampleProfilerSession.cs b/src/Sentry.Profiling/SampleProfilerSession.cs index 91eba73fe2..38f2069f5e 100644 --- a/src/Sentry.Profiling/SampleProfilerSession.cs +++ b/src/Sentry.Profiling/SampleProfilerSession.cs @@ -61,13 +61,7 @@ private SampleProfilerSession(SentryStopwatch stopwatch, EventPipeSession sessio internal static bool ThrowOnNextStartupForTests { get { return _throwOnNextStartupForTests; } - set - { - if (value) - _throwOnNextStartupForTests.CompareExchange(true, false); - else - _throwOnNextStartupForTests.CompareExchange(false, true); - } + set { _throwOnNextStartupForTests = value; } } public static SampleProfilerSession StartNew(IDiagnosticLogger? logger = null) diff --git a/test/Sentry.Tests/Internals/InterlockedBooleanTests.cs b/test/Sentry.Tests/Internals/InterlockedBooleanTests.cs index ff2b1cf50e..49c967a30b 100644 --- a/test/Sentry.Tests/Internals/InterlockedBooleanTests.cs +++ b/test/Sentry.Tests/Internals/InterlockedBooleanTests.cs @@ -96,7 +96,7 @@ public void InterlockedBoolean_Exchange_SetsExpectedNewState(bool initialState, var expected = ToTBool(newValue); // Act - var result = sut.Exchange(newValue); + var _ = sut.Exchange(newValue); // Assert sut.ValueForTests.Should().Be(expected); From 71fc795107b11363cb6cb581482146e8ff439096 Mon Sep 17 00:00:00 2001 From: Jonathan Gilbert Date: Wed, 8 Oct 2025 20:33:06 -0500 Subject: [PATCH 09/10] Use .Exchange instead of assignment for an automated test-related InterlockedBoolean in SampleProfilerSection.cs in case the assignment isn't atomic. --- src/Sentry.Profiling/SampleProfilerSession.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sentry.Profiling/SampleProfilerSession.cs b/src/Sentry.Profiling/SampleProfilerSession.cs index 38f2069f5e..f3de9c2186 100644 --- a/src/Sentry.Profiling/SampleProfilerSession.cs +++ b/src/Sentry.Profiling/SampleProfilerSession.cs @@ -61,7 +61,7 @@ private SampleProfilerSession(SentryStopwatch stopwatch, EventPipeSession sessio internal static bool ThrowOnNextStartupForTests { get { return _throwOnNextStartupForTests; } - set { _throwOnNextStartupForTests = value; } + set { _throwOnNextStartupForTests.Exchange(value); } } public static SampleProfilerSession StartNew(IDiagnosticLogger? logger = null) From 3cc3faf11ccd32918989a7fa605e9a2cd753d895 Mon Sep 17 00:00:00 2001 From: Jonathan Gilbert Date: Fri, 10 Oct 2025 11:02:23 -0500 Subject: [PATCH 10/10] Moved the .editorconfig configuration for using directive placement from the file local to Sentry.Azure.Functions.Worker to the root file, and changed it to csharp_using_directive_placement. --- .editorconfig | 3 +++ src/Sentry.Azure.Functions.Worker/.editorconfig | 4 +--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.editorconfig b/.editorconfig index a2a479130e..01deda3d42 100644 --- a/.editorconfig +++ b/.editorconfig @@ -40,6 +40,9 @@ indent_size = 2 # Sort using and Import directives with System.* appearing first dotnet_sort_system_directives_first = true +# Keep using directives at the top of the file, outside of the namespace +csharp_using_directive_placement = outside_namespace + # Avoid "this." and "Me." if not necessary dotnet_style_qualification_for_field = false : warning dotnet_style_qualification_for_property = false : warning diff --git a/src/Sentry.Azure.Functions.Worker/.editorconfig b/src/Sentry.Azure.Functions.Worker/.editorconfig index 75b15d3f30..bdf468a081 100644 --- a/src/Sentry.Azure.Functions.Worker/.editorconfig +++ b/src/Sentry.Azure.Functions.Worker/.editorconfig @@ -2,6 +2,4 @@ # Reason: Azure Functions worker doesn't set SynchronizationContext but Durable Functions does and required affinity. # (https://github.com/Azure/azure-functions-dotnet-worker/issues/1520) -dotnet_diagnostic.CA2007.severity = none - -dotnet_style_namespace_declaration_with_usings_placement = prefer_outside_namespace +dotnet_diagnostic.CA2007.severity = none \ No newline at end of file