Skip to content

Commit

Permalink
Fix a data race for SentryId.empty (#3072)
Browse files Browse the repository at this point in the history
Initialize the SentryId.empty in the class initializer to fix a data race.
  • Loading branch information
philipphofmann authored Jun 5, 2023
1 parent 6943de0 commit f0283e8
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

### Fixes

- Fix a data race for `SentryId.empty` (#3072)
- Changed `Trace` serialized value of `sampled` from string to boolean (#3067)
- Duplicated HTTP breadcrumbs (#3058)
- Expose SentryPrivate and SentrySwiftUI schemes for cartahge clients that have `--no-use-binaries` option (#3071)
Expand Down
13 changes: 10 additions & 3 deletions Sources/Sentry/SentryId.m
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@ @implementation SentryId

static SentryId *_empty = nil;

+ (void)initialize
{
if (self == [SentryId class]) {
// Initialize the empty here instead of using a lazy load approach in the getter to avoid
// using a lock to prevent data races. Empty is used at a couple of places in the SDK, so
// potentially minimizing the memory footprint doesn't apply.
_empty = [[SentryId alloc] initWithUUIDString:emptyUUIDString];
}
}

- (instancetype)init
{
return [self initWithUUID:[NSUUID UUID]];
Expand Down Expand Up @@ -84,9 +94,6 @@ - (NSUInteger)hash

+ (SentryId *)empty
{
if (nil == _empty) {
_empty = [[SentryId alloc] initWithUUIDString:emptyUUIDString];
}
return _empty;
}

Expand Down
6 changes: 6 additions & 0 deletions Tests/SentryTests/Protocol/SentryIdTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,10 @@ class SentryIdTests: XCTestCase {
func testHash_IsDifferentWhenObjectsAreDifferent() {
XCTAssertNotEqual(SentryId(uuid: UUID()).hash, SentryId(uuid: fixture.uuid).hash)
}

func testConcurrentReadsOfEmpty() {
testConcurrentModifications { _ in
XCTAssertNotNil(SentryId.empty)
}
}
}

0 comments on commit f0283e8

Please sign in to comment.