From ff27daf1805759bf128e129ba281ece3128482b9 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Wed, 16 Nov 2022 19:26:51 +0100 Subject: [PATCH 1/2] fix: Crash in Client when reading integrations Fix reading from the integrations list while it's being modified on another thread on the client. Fixes GH-2397 --- CHANGELOG.md | 6 ++++++ Sources/Sentry/SentryClient.m | 8 +++++++- Tests/SentryTests/SentryClientTests.swift | 23 +++++++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c927e510d3..770413bece3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- Crash in Client when reading integrations (#2397) + ## 7.31.1 ### Fixes diff --git a/Sources/Sentry/SentryClient.m b/Sources/Sentry/SentryClient.m index 3da3f62dc2e..cc3fbf81148 100644 --- a/Sources/Sentry/SentryClient.m +++ b/Sources/Sentry/SentryClient.m @@ -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. + for (NSString *integration in SentrySDK.currentHub.installedIntegrationNames.copy) { // 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" diff --git a/Tests/SentryTests/SentryClientTests.swift b/Tests/SentryTests/SentryClientTests.swift index a9acfa07fed..1bf32e96b18 100644 --- a/Tests/SentryTests/SentryClientTests.swift +++ b/Tests/SentryTests/SentryClientTests.swift @@ -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() From 31237bc449b7042ff3235335aa9b106f27d1e763 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Wed, 16 Nov 2022 19:28:20 +0100 Subject: [PATCH 2/2] Fix changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 770413bece3..8ba40e1e63f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixes -- Crash in Client when reading integrations (#2397) +- Crash in Client when reading integrations (#2398) ## 7.31.1