Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
ref(logs): no longer enter Counter-Scopes when a Lock-Scope has been …
…entered successfully
  • Loading branch information
Flash0ver committed Aug 11, 2025
commit 0c75df7b74109324f7343c0f693953f0e6ec19b5
11 changes: 8 additions & 3 deletions src/Sentry/Threading/ScopedCountdownLock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,16 @@ internal ScopedCountdownLock()
/// Gets the number of remaining <see cref="CounterScope"/> required to exit in order to set/signal the event while a <see cref="LockScope"/> is active.
/// When <see langword="0"/> and while a <see cref="LockScope"/> is active, no more <see cref="CounterScope"/> can be entered.
/// </summary>
internal int Count => _isEngaged == 1 ? _event.CurrentCount : _event.CurrentCount - 1;
internal int Count => IsEngaged ? _event.CurrentCount : _event.CurrentCount - 1;

/// <summary>
/// Returns <see langword="true"/> when a <see cref="LockScope"/> is active and the event can be set/signaled by <see cref="Count"/> reaching <see langword="0"/>.
/// Returns <see langword="false"/> when the <see cref="Count"/> can only reach the initial count of <see langword="1"/> when no <see cref="CounterScope"/> is active any longer.
/// </summary>
internal bool IsEngaged => _isEngaged == 1;
internal bool IsEngaged => Interlocked.CompareExchange(ref _isEngaged, 1, 1) == 1;

/// <summary>
/// No <see cref="CounterScope"/> will be entered when the <see cref="Count"/> has reached <see langword="0"/> while the lock is engaged via an active <see cref="LockScope"/>.
/// No <see cref="CounterScope"/> will be entered when the <see cref="Count"/> has reached <see langword="0"/>, or while the lock is engaged via an active <see cref="LockScope"/>.
/// Check via <see cref="CounterScope.IsEntered"/> whether the underlying <see cref="CountdownEvent"/> has not been set/signaled yet.
/// To signal the underlying <see cref="CountdownEvent"/>, ensure <see cref="CounterScope.Dispose"/> is called.
/// </summary>
Expand All @@ -49,6 +49,11 @@ internal ScopedCountdownLock()
/// </remarks>
internal CounterScope TryEnterCounterScope()
{
if (IsEngaged)
Copy link
Member Author

@Flash0ver Flash0ver Aug 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: additional change

no longer allowing to Add new logs to a particular Buffer, while a Flush has been requested (in this case occurring during the 5 second timeout), which may have previously "dragged out" the Flush by still allowing new Add operations before the Event is actually set by reaching a count of 0 (while the buffer is not full yet)

this also effects #4425, where - in the case of a terminating unhandled exception - no more Add operations are admitted and we only wait until all currently in-progress Add operations have concluded in order to do a "safe" Flush, which should complete quite quickly, not "unreasonably" dragging out the shutdown

{
return new CounterScope();
}

if (_event.TryAddCount(1))
{
return new CounterScope(this);
Expand Down
14 changes: 7 additions & 7 deletions test/Sentry.Tests/Threading/ScopedCountdownLockTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,20 +79,20 @@ public void TryEnterLockScope_IsEngaged_IsSet()
lockTwo.Dispose();
AssertEngaged(false, 1);

// successfully enter another CounterScope ... lock is engaged but not yet set
// cannot enter another CounterScope as long as the lock is already engaged by a LockScope
var counterTwo = _lock.TryEnterCounterScope();
counterTwo.IsEntered.Should().BeTrue();
AssertEngaged(false, 2);
counterTwo.IsEntered.Should().BeFalse();
AssertEngaged(false, 1);

// exit a CounterScope ... decrement the count
// no-op ... CounterScope is not entered
counterTwo.Dispose();
AssertEngaged(false, 1);

// exit last CounterScope ... count of engaged lock reaches zero ... sets the lock
counterOne.Dispose();
AssertEngaged(true, 0);

// cannot enter another CounterScope as long as the engaged lock is set
// cannot enter another CounterScope as long as the lock is engaged and set
var counterThree = _lock.TryEnterCounterScope();
counterThree.IsEntered.Should().BeFalse();
AssertEngaged(true, 0);
Expand All @@ -102,11 +102,11 @@ public void TryEnterLockScope_IsEngaged_IsSet()
// would block if the count of the engaged lock was not zero
lockOne.Wait();

// exit the LockScope ... reset the lock
// exit the LockScope ... reset and disengage the lock
lockOne.Dispose();
AssertDisengaged(false, 0);

// can enter a CounterScope again ... the lock not set
// can enter a CounterScope again ... the lock is no longer set and no longer engaged
var counterFour = _lock.TryEnterCounterScope();
counterFour.IsEntered.Should().BeTrue();
AssertDisengaged(false, 1);
Expand Down