Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Crash in Client when reading integrations #2398

Merged
merged 11 commits into from
Nov 18, 2022
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixes

- Crash in Client when reading integrations (#2398)

## 7.31.1

### Fixes
Expand Down
8 changes: 7 additions & 1 deletion Sources/Sentry/SentryClient.m
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,13 @@ - (void)setSdk:(SentryEvent *)event
id integrations = event.extra[@"__sentry_sdk_integrations"];
if (!integrations) {
integrations = [NSMutableArray new];
for (NSString *integration in SentrySDK.currentHub.installedIntegrationNames) {

// The SDK can send an event before it installs all integrations, which add to this list. To
// avoid an exception while iterating over a list while items get added to it, we copy it.
// With that approach, we might not have the complete integration list. To fix this, the SDK
// would need to ask all integrations first if they should be installed before actually
// installing them, which would be quite a bit a of a refactoring.
philipphofmann marked this conversation as resolved.
Show resolved Hide resolved
for (NSString *integration in SentrySDK.currentHub.installedIntegrationNames.copy) {
philipphofmann marked this conversation as resolved.
Show resolved Hide resolved
// Every integration starts with "Sentry" and ends with "Integration". To keep the
// payload of the event small we remove both.
NSString *withoutSentry = [integration stringByReplacingOccurrencesOfString:@"Sentry"
Expand Down
23 changes: 23 additions & 0 deletions Tests/SentryTests/SentryClientTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1264,6 +1264,29 @@ class SentryClientTest: XCTestCase {
XCTAssertEqual(item, fixture.transportAdapter.sendEventWithTraceStateInvocations.first?.additionalEnvelopeItems.first)
}

@available(iOS 13.0, tvOS 13.0, OSX 10.15, * )
func testConcurrentlyAddingInstalledIntegrations_WhileSendingEvents() async {
let sut = fixture.getSut()

let hub = SentryHub(client: sut, andScope: nil)
SentrySDK.setCurrentHub(hub)

// Run this in a loop to ensure that add while iterating over the integrations
// Running it once doesn't guaranty failure
for _ in 0..<100 {
let install = Task {
for i in 0..<10_000 {
hub.installedIntegrationNames.add("Integration\(i)")
}
}

sut.capture(event: fixture.event)
install.cancel()
await install.value
hub.installedIntegrationNames.removeAllObjects()
}
}

private func givenEventWithDebugMeta() -> Event {
let event = Event(level: SentryLevel.fatal)
let debugMeta = DebugMeta()
Expand Down