Skip to content

Commit ffe3780

Browse files
authored
fix: protect UnsampledTransaction against double-Dispose (#4722)
1 parent 5837b9f commit ffe3780

File tree

2 files changed

+46
-4
lines changed

2 files changed

+46
-4
lines changed

src/Sentry/Internal/UnsampledTransaction.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ internal sealed class UnsampledTransaction : NoOpTransaction
2121
private readonly IHub _hub;
2222
private readonly ITransactionContext _context;
2323
private readonly SentryOptions? _options;
24+
private InterlockedBoolean _isFinished;
2425

2526
public UnsampledTransaction(IHub hub, ITransactionContext context)
2627
{
@@ -32,7 +33,6 @@ public UnsampledTransaction(IHub hub, ITransactionContext context)
3233

3334
internal DynamicSamplingContext? DynamicSamplingContext { get; set; }
3435

35-
private bool _isFinished;
3636
public override bool IsFinished => _isFinished;
3737

3838
public override IReadOnlyCollection<ISpan> Spans => _spans;
@@ -63,11 +63,14 @@ public override string Operation
6363

6464
public override void Finish()
6565
{
66-
_options?.LogDebug("Finishing unsampled transaction");
67-
6866
// Ensure the transaction is really cleared from the scope
6967
// See: https://github.com/getsentry/sentry-dotnet/issues/4198
70-
_isFinished = true;
68+
if (_isFinished.Exchange(true))
69+
{
70+
return;
71+
}
72+
73+
_options?.LogDebug("Finishing unsampled transaction");
7174

7275
// Clear the transaction from the scope and regenerate the Propagation Context, so new events don't have a
7376
// trace context that is "older" than the transaction that just finished

test/Sentry.Tests/Internals/UnsampledTransactionTests.cs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,45 @@ namespace Sentry.Tests.Internals;
22

33
public class UnsampledTransactionTests
44
{
5+
[Fact]
6+
public void Dispose_Unfinished_Finishes()
7+
{
8+
// Arrange
9+
var hub = Substitute.For<IHub>();
10+
ITransactionContext context = new TransactionContext("TestTransaction", "TestOperation",
11+
new SentryTraceHeader(SentryId.Create(), SpanId.Create(), false)
12+
);
13+
var transaction = new UnsampledTransaction(hub, context);
14+
15+
// Act
16+
transaction.Dispose();
17+
18+
// Assert
19+
transaction.IsFinished.Should().BeTrue();
20+
hub.Received(1).ConfigureScope(Arg.Any<Action<Scope, UnsampledTransaction>>(),
21+
Arg.Is<UnsampledTransaction>(t => ReferenceEquals(t, transaction)));
22+
}
23+
24+
[Fact]
25+
public void Dispose_Finished_DoesNothing()
26+
{
27+
// Arrange
28+
var hub = Substitute.For<IHub>();
29+
ITransactionContext context = new TransactionContext("TestTransaction", "TestOperation",
30+
new SentryTraceHeader(SentryId.Create(), SpanId.Create(), false)
31+
);
32+
var transaction = new UnsampledTransaction(hub, context);
33+
transaction.Finish();
34+
35+
// Act
36+
transaction.Dispose();
37+
38+
// Assert
39+
transaction.IsFinished.Should().BeTrue();
40+
hub.Received(1).ConfigureScope(Arg.Any<Action<Scope, UnsampledTransaction>>(),
41+
Arg.Is<UnsampledTransaction>(t => ReferenceEquals(t, transaction)));
42+
}
43+
544
[Fact]
645
public void StartChild_CreatesSpan_IsTrackedByParent()
746
{

0 commit comments

Comments
 (0)