Skip to content

Commit

Permalink
fix: Make SentryScope.useSpan non-blocking
Browse files Browse the repository at this point in the history
Instead of calling the callback of useSpan within the spanLock, get a
local copy of the span and pass this to the callback. This way, when the
callback is a blocking call, useSpan doesn't block other invocations of
itself and prevents potential deadlocks.
  • Loading branch information
philipphofmann committed Jan 23, 2024
1 parent 42ef6ba commit 70c017e
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

- Fix typo in BUILD_LIBRARY_FOR_DISTRIBUTION variable in Makefile (#3488)
- Remove dispatch queue metadata collection to fix crash (#3522)
- Make SentryScope.useSpan non-blocking (#3568)

## 8.18.0

Expand Down
4 changes: 3 additions & 1 deletion Sources/Sentry/SentryScope.m
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,11 @@ - (void)setSpan:(nullable id<SentrySpan>)span

- (void)useSpan:(SentrySpanCallback)callback
{
id<SentrySpan> localSpan = nil;
@synchronized(_spanLock) {
callback(_span);
localSpan = _span;
}
callback(localSpan);
}

- (void)clear
Expand Down
43 changes: 43 additions & 0 deletions Tests/SentryTests/SentryScopeSwiftTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,49 @@ class SentryScopeSwiftTests: XCTestCase {
}
}

func testUseSpanLock_DoesNotBlock_WithBlockingCallback() {
let scope = fixture.scope
let queue = DispatchQueue(label: "test-queue", attributes: [.initiallyInactive, .concurrent])
let expect = expectation(description: "useSpan callback is non-blocking")

let condition = NSCondition()
var useSpanCalled = false

queue.async {
scope.useSpan { _ in
condition.lock()
while !useSpanCalled {
condition.wait()
}
condition.unlock()
}
}

queue.async {
scope.useSpan { _ in
useSpanCalled = true
condition.broadcast()
expect.fulfill()
}
}

queue.activate()

wait(for: [expect], timeout: 0.1)
}

func testUseSpanLock_IsReentrant() {
let expect = expectation(description: "finish on time")
let scope = fixture.scope
scope.useSpan { _ in
scope.useSpan { _ in
expect.fulfill()
}

}
wait(for: [expect], timeout: 0.1)
}

func testUseSpanForClear() {
fixture.scope.span = fixture.transaction
fixture.scope.useSpan { (_) in
Expand Down

0 comments on commit 70c017e

Please sign in to comment.