diff --git a/CHANGELOG.md b/CHANGELOG.md index 288c40e20a..43aef3d7de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Unreleased +- [FIX] Fix race condition during consent change, preventing loss of events recorded on the current thread. See [#2063][] - [IMPROVEMENT] Support mutation of events' attributes. See [#2099][] # 2.19.0 / 28-10-2024 @@ -786,6 +787,7 @@ Release `2.0` introduces breaking changes. Follow the [Migration Guide](MIGRATIO [#2088]: https://github.com/DataDog/dd-sdk-ios/pull/2088 [#2083]: https://github.com/DataDog/dd-sdk-ios/pull/2083 [#2099]: https://github.com/DataDog/dd-sdk-ios/pull/2099 +[#2063]: https://github.com/DataDog/dd-sdk-ios/pull/2063 [@00fa9a]: https://github.com/00FA9A [@britton-earnin]: https://github.com/Britton-Earnin [@hengyu]: https://github.com/Hengyu diff --git a/DatadogCore/Sources/Core/DatadogCore.swift b/DatadogCore/Sources/Core/DatadogCore.swift index 2bc02b2753..189f7280b4 100644 --- a/DatadogCore/Sources/Core/DatadogCore.swift +++ b/DatadogCore/Sources/Core/DatadogCore.swift @@ -160,7 +160,12 @@ internal final class DatadogCore { /// - Parameter trackingConsent: new consent value, which will be applied for all data collected from now on func set(trackingConsent: TrackingConsent) { if trackingConsent != consentPublisher.consent { - allStorages.forEach { $0.migrateUnauthorizedData(toConsent: trackingConsent) } + contextProvider.queue.async { [allStorages] in + // RUM-3175: To prevent race conditions with ongoing "event write" operations, + // data migration must be synchronized on the context queue. This guarantees that + // all latest events have been written before migration occurs. + allStorages.forEach { $0.migrateUnauthorizedData(toConsent: trackingConsent) } + } consentPublisher.consent = trackingConsent } } diff --git a/DatadogCore/Tests/Datadog/DatadogCore/DatadogCoreTests.swift b/DatadogCore/Tests/Datadog/DatadogCore/DatadogCoreTests.swift index 9a51a52d7f..acf94fe544 100644 --- a/DatadogCore/Tests/Datadog/DatadogCore/DatadogCoreTests.swift +++ b/DatadogCore/Tests/Datadog/DatadogCore/DatadogCoreTests.swift @@ -78,6 +78,65 @@ class DatadogCoreTests: XCTestCase { XCTAssertEqual(requestBuilderSpy.requestParameters.count, 1, "It should send only one request") } + func testWhenWritingEventsWithPendingConsentThenGranted_itUploadsAllEvents() throws { + // Given + let core = DatadogCore( + directory: temporaryCoreDirectory, + dateProvider: SystemDateProvider(), + initialConsent: .mockRandom(), + performance: .combining( + storagePerformance: StoragePerformanceMock.readAllFiles, + uploadPerformance: UploadPerformanceMock.veryQuick + ), + httpClient: HTTPClientMock(), + encryption: nil, + contextProvider: .mockAny(), + applicationVersion: .mockAny(), + maxBatchesPerUpload: 1, + backgroundTasksEnabled: .mockAny() + ) + defer { core.flushAndTearDown() } + + let send2RequestsExpectation = expectation(description: "send 2 requests") + send2RequestsExpectation.expectedFulfillmentCount = 2 + + let requestBuilderSpy = FeatureRequestBuilderSpy() + requestBuilderSpy.onRequest = { _, _ in send2RequestsExpectation.fulfill() } + + try core.register(feature: FeatureMock(requestBuilder: requestBuilderSpy)) + + // When + let scope = core.scope(for: FeatureMock.self) + core.set(trackingConsent: .pending) + scope.eventWriteContext { context, writer in + XCTAssertEqual(context.trackingConsent, .pending) + writer.write(value: FeatureMock.Event(event: "pending")) + } + + core.set(trackingConsent: .granted) + scope.eventWriteContext { context, writer in + XCTAssertEqual(context.trackingConsent, .granted) + writer.write(value: FeatureMock.Event(event: "granted")) + } + + // Then + waitForExpectations(timeout: 2) + + let uploadedEvents = requestBuilderSpy.requestParameters + .flatMap { $0.events } + .map { $0.data.utf8String } + + XCTAssertEqual( + uploadedEvents, + [ + #"{"event":"pending"}"#, + #"{"event":"granted"}"# + ], + "It should upload all events" + ) + XCTAssertEqual(requestBuilderSpy.requestParameters.count, 2, "It should send 2 requests") + } + func testWhenWritingEventsWithBypassingConsent_itUploadsAllEvents() throws { // Given let core = DatadogCore( diff --git a/DatadogCore/Tests/Datadog/Mocks/DatadogInternal/UploadMock.swift b/DatadogCore/Tests/Datadog/Mocks/DatadogInternal/UploadMock.swift index bc20a3704a..7b8f9e4f97 100644 --- a/DatadogCore/Tests/Datadog/Mocks/DatadogInternal/UploadMock.swift +++ b/DatadogCore/Tests/Datadog/Mocks/DatadogInternal/UploadMock.swift @@ -29,15 +29,21 @@ internal class FeatureRequestBuilderMock: FeatureRequestBuilder { } internal class FeatureRequestBuilderSpy: FeatureRequestBuilder { - /// Records parameters passed to `requet(for:with:)` + /// Stores the parameters passed to the `request(for:with:)` method. + @ReadWriteLock private(set) var requestParameters: [(events: [Event], context: DatadogContext)] = [] + /// A closure that is called when a request is about to be created in the `request(for:with:)` method. + @ReadWriteLock + var onRequest: ((_ events: [Event], _ context: DatadogContext) -> Void)? + func request( for events: [Event], with context: DatadogContext, execution: ExecutionContext ) throws -> URLRequest { requestParameters.append((events: events, context: context)) + onRequest?(events, context) return .mockAny() } }