Skip to content

Commit

Permalink
fix: Threading issues in binary image cache (#3468)
Browse files Browse the repository at this point in the history
Add synchronize to start and stop of SentryBinaryImage crash to avoid
threading issues that can lead to crashes.

Fixes GH-3462
  • Loading branch information
philipphofmann authored Dec 1, 2023
1 parent 24c239b commit 5ab8ec2
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

- Crash when UINavigationController doesn't have rootViewController (#3455)
- Crash when synchronizing invalid JSON breadcrumbs to SentryWatchdogTermination (#3458)
- Threading issues in binary image cache (#3468)
- Finish transaction for external view controllers (#3440)

## 8.17.0
Expand Down
16 changes: 10 additions & 6 deletions Sources/Sentry/SentryBinaryImageCache.m
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,20 @@ @implementation SentryBinaryImageCache

- (void)start
{
_cache = [NSMutableArray array];
sentrycrashbic_registerAddedCallback(&binaryImageWasAdded);
sentrycrashbic_registerRemovedCallback(&binaryImageWasRemoved);
@synchronized(self) {
_cache = [NSMutableArray array];
sentrycrashbic_registerAddedCallback(&binaryImageWasAdded);
sentrycrashbic_registerRemovedCallback(&binaryImageWasRemoved);
}
}

- (void)stop
{
sentrycrashbic_registerAddedCallback(NULL);
sentrycrashbic_registerRemovedCallback(NULL);
_cache = nil;
@synchronized(self) {
sentrycrashbic_registerAddedCallback(NULL);
sentrycrashbic_registerRemovedCallback(NULL);
_cache = nil;
}
}

- (void)binaryImageAdded:(const SentryCrashBinaryImage *)image
Expand Down
20 changes: 20 additions & 0 deletions Tests/SentryTests/SentryBinaryImageCacheTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,26 @@ class SentryBinaryImageCacheTests: XCTestCase {
let didNotFind = sut.pathFor(inAppInclude: "Name at 0")
expect(didNotFind) == nil
}

func testAddingImagesWhileStoppingAndStartingOnDifferentThread() {
let count = 1_000

let expectation = expectation(description: "Add images on background thread")
expectation.expectedFulfillmentCount = count

for i in 0..<count {
DispatchQueue.global().async {
var binaryImage0 = self.createCrashBinaryImage(UInt(i * 10))
self.sut.binaryImageAdded(&binaryImage0)

self.sut.stop()
self.sut.start()
expectation.fulfill()
}
}

waitForExpectations(timeout: 1)
}

func createCrashBinaryImage(_ address: UInt) -> SentryCrashBinaryImage {
let name = "Expected Name at \(address)"
Expand Down

0 comments on commit 5ab8ec2

Please sign in to comment.