From 05b56287f67f241f4f620e55c3a831a2f7872c8e Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Fri, 18 Sep 2026 18:29:12 -0700 Subject: [PATCH 1/4] feat(events): add flushAndWait on Apple (tier 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bounded flush spends up to a caller-supplied budget trying to deliver, then reports whether the events left the SDK's hands. Android already had this; Apple now has LDClient.flushAndWait(timeout:), spelled in seconds to match identify and start. EventReporting.flushReportingOutcome is the seam: true when the events were delivered, refused for good, or there were none; false when the SDK is offline or a retryable failure means they are still waiting. Saying that needed the response classification to be a decision rather than a Bool, so processEventResponse now returns settled, retryable, or dropped -- the three outcomes that were already implied by its two callers. TimeoutExecutor bounds the wait. Completions run on a queue that is neither main nor the reporter's delivery queue, so a lifecycle caller on the main thread cannot deadlock waiting for itself. Backgrounding now spends its last moment trying to deliver, inside a ProcessInfo activity assertion so the system does not suspend the process out from under a request that has only just reached the network. This is not a crash-time API. The SDK has no crash hook of its own on Apple, and a Mach exception handler is not a supported caller. Spec: Event Durability §10, §11, §17.3. Co-authored-by: Cursor --- LaunchDarkly.xcodeproj/project.pbxproj | 10 ++ .../GeneratedCode/mocks.generated.swift | 11 +- LaunchDarkly/LaunchDarkly/LDClient.swift | 132 ++++++++++++++---- .../ObjectiveC/ObjcLDClient.swift | 14 ++ .../ServiceObjects/BackgroundActivity.swift | 46 ++++++ .../ServiceObjects/EventReporter.swift | 61 +++++--- .../LaunchDarklyTests/LDClientSpec.swift | 45 ++++++ .../Mocks/ClientServiceMockFactory.swift | 9 +- SourceryTemplates/mocks.stencil | 2 +- 9 files changed, 285 insertions(+), 45 deletions(-) create mode 100644 LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift diff --git a/LaunchDarkly.xcodeproj/project.pbxproj b/LaunchDarkly.xcodeproj/project.pbxproj index d4feb3f0b..94c6b3b21 100644 --- a/LaunchDarkly.xcodeproj/project.pbxproj +++ b/LaunchDarkly.xcodeproj/project.pbxproj @@ -32,6 +32,7 @@ 3D2406222E0D90E000F91253 /* EnvironmentMetadata.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3D24060F2E0D90E000F91253 /* EnvironmentMetadata.swift */; }; 3D2406232E0D90E000F91253 /* SdkMetadata.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3D2406122E0D90E000F91253 /* SdkMetadata.swift */; }; 3D2406252E0D90EA00F91253 /* LDClientPluginsSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3D2406242E0D90EA00F91253 /* LDClientPluginsSpec.swift */; }; + 3D34EEEF37158657D49335BB /* BackgroundActivity.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9A771B316E19ED598CD4FB0B /* BackgroundActivity.swift */; }; 3D3AB9432A4F16FE003AECF1 /* ReportingConsts.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3D3AB9422A4F16FE003AECF1 /* ReportingConsts.swift */; }; 3D3AB9442A4F16FE003AECF1 /* ReportingConsts.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3D3AB9422A4F16FE003AECF1 /* ReportingConsts.swift */; }; 3D3AB9452A4F16FE003AECF1 /* ReportingConsts.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3D3AB9422A4F16FE003AECF1 /* ReportingConsts.swift */; }; @@ -45,6 +46,7 @@ 50EE85C42EA0487F007CC662 /* TimeoutExecutor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50EE85C12EA0487F007CC662 /* TimeoutExecutor.swift */; }; 50EE85C52EA0487F007CC662 /* TimeoutExecutor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50EE85C12EA0487F007CC662 /* TimeoutExecutor.swift */; }; 50EE85C72EA0749C007CC662 /* TimeoutExecutorSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50EE85C62EA0749C007CC662 /* TimeoutExecutorSpec.swift */; }; + 7185A3C30E7FFC4F725769F1 /* BackgroundActivity.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9A771B316E19ED598CD4FB0B /* BackgroundActivity.swift */; }; 830BF933202D188E006DF9B1 /* HTTPURLRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 830BF932202D188E006DF9B1 /* HTTPURLRequest.swift */; }; 830DB3AC22380A3E00D65D25 /* HTTPHeadersSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = 830DB3AB22380A3E00D65D25 /* HTTPHeadersSpec.swift */; }; 830DB3AE2239B54900D65D25 /* URLResponse.swift in Sources */ = {isa = PBXBuildFile; fileRef = 830DB3AD2239B54900D65D25 /* URLResponse.swift */; }; @@ -238,6 +240,7 @@ 9BE9637664F4E97681CFCD45 /* EventJSONWriter.swift in Sources */ = {isa = PBXBuildFile; fileRef = B430884DBF2E541B919C08FA /* EventJSONWriter.swift */; }; A032A73E446055D14A546CA0 /* EventJSONWriter.swift in Sources */ = {isa = PBXBuildFile; fileRef = B430884DBF2E541B919C08FA /* EventJSONWriter.swift */; }; A0BAD9EE25943C43A5154E6F /* EventJSONWriterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 367A9182439001C3EBD68477 /* EventJSONWriterTests.swift */; }; + A20146E7AF098560627C614E /* BackgroundActivity.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9A771B316E19ED598CD4FB0B /* BackgroundActivity.swift */; }; A3047D642A606B6000F568E0 /* SDKEnvironmentReporterSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = A3047D5D2A606B6000F568E0 /* SDKEnvironmentReporterSpec.swift */; }; A3047D652A606B6000F568E0 /* IOSEnvironmentReporterSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = A3047D5E2A606B6000F568E0 /* IOSEnvironmentReporterSpec.swift */; }; A3047D662A606B6000F568E0 /* EnvironmentReporterChainBaseSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = A3047D5F2A606B6000F568E0 /* EnvironmentReporterChainBaseSpec.swift */; }; @@ -261,6 +264,7 @@ A31088272837DCA900184942 /* LDContextSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = A31088242837DCA900184942 /* LDContextSpec.swift */; }; A31088282837DCA900184942 /* ReferenceSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = A31088252837DCA900184942 /* ReferenceSpec.swift */; }; A31088292837DCA900184942 /* KindSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = A31088262837DCA900184942 /* KindSpec.swift */; }; + A32FCDB22D53263851DDB727 /* BackgroundActivity.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9A771B316E19ED598CD4FB0B /* BackgroundActivity.swift */; }; A33A5F7A28466D04000C29C7 /* LDContextStub.swift in Sources */ = {isa = PBXBuildFile; fileRef = A33A5F7928466D04000C29C7 /* LDContextStub.swift */; }; A3470C372B7C1ACE00951CEE /* LDValueDecoder.swift in Sources */ = {isa = PBXBuildFile; fileRef = A3470C362B7C1ACE00951CEE /* LDValueDecoder.swift */; }; A3470C382B7C1ACE00951CEE /* LDValueDecoder.swift in Sources */ = {isa = PBXBuildFile; fileRef = A3470C362B7C1ACE00951CEE /* LDValueDecoder.swift */; }; @@ -565,6 +569,7 @@ 88DAB8D1864C14D9C7E06F11 /* LDContextJSONWriter.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = LDContextJSONWriter.swift; sourceTree = ""; }; 9A47F7492F3D4CBF0001FD9C /* ContextSummarizerSpec.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContextSummarizerSpec.swift; sourceTree = ""; }; 9A47F74B2F3D4CCF0001FD9C /* ContextSummarizer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContextSummarizer.swift; sourceTree = ""; }; + 9A771B316E19ED598CD4FB0B /* BackgroundActivity.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = BackgroundActivity.swift; sourceTree = ""; }; 9AE1D0A02F4A1B000001FD9C /* EvaluationExposureDeduper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EvaluationExposureDeduper.swift; sourceTree = ""; }; 9AE1D0A52F4A1B000001FD9C /* EvaluationExposureDeduperSpec.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EvaluationExposureDeduperSpec.swift; sourceTree = ""; }; 9AE1D0B02F4A1B000001FD9C /* HookDecorator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HookDecorator.swift; sourceTree = ""; }; @@ -997,6 +1002,7 @@ 831AAE2B20A9E4F600B46DBA /* Throttler.swift */, B430884DBF2E541B919C08FA /* EventJSONWriter.swift */, D93B7F5F6CF5C3DE79FB4286 /* ContextEncodingCache.swift */, + 9A771B316E19ED598CD4FB0B /* BackgroundActivity.swift */, ); path = ServiceObjects; sourceTree = ""; @@ -1519,6 +1525,7 @@ A8449C0800DF492B8F0619E4 /* LDContextJSONWriter.swift in Sources */, 94C1C136BA8989F1B8AFE0A8 /* EventJSONWriter.swift in Sources */, 40F4D07AD934AF976687011D /* ContextEncodingCache.swift in Sources */, + A32FCDB22D53263851DDB727 /* BackgroundActivity.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -1609,6 +1616,7 @@ DAF5695D58E28B8CF9221213 /* LDContextJSONWriter.swift in Sources */, C4AF311E4DA8C3AB7040637D /* EventJSONWriter.swift in Sources */, 4882B223C23B63F30BC60941 /* ContextEncodingCache.swift in Sources */, + 7185A3C30E7FFC4F725769F1 /* BackgroundActivity.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -1698,6 +1706,7 @@ AFCE752CB1A75DEBABC23034 /* LDContextJSONWriter.swift in Sources */, 9BE9637664F4E97681CFCD45 /* EventJSONWriter.swift in Sources */, D7A500FCA04E3412F931003F /* ContextEncodingCache.swift in Sources */, + A20146E7AF098560627C614E /* BackgroundActivity.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -1850,6 +1859,7 @@ EBFA09F4DFC59FCB6C0AE209 /* LDContextJSONWriter.swift in Sources */, A032A73E446055D14A546CA0 /* EventJSONWriter.swift in Sources */, BAE0B4F45562EBF4AC786B5D /* ContextEncodingCache.swift in Sources */, + 3D34EEEF37158657D49335BB /* BackgroundActivity.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/LaunchDarkly/GeneratedCode/mocks.generated.swift b/LaunchDarkly/GeneratedCode/mocks.generated.swift index 6dd5106aa..d8cc0b4e5 100644 --- a/LaunchDarkly/GeneratedCode/mocks.generated.swift +++ b/LaunchDarkly/GeneratedCode/mocks.generated.swift @@ -221,6 +221,15 @@ final class EventReportingMock: EventReporting { flushReceivedCompletion = completion try! flushCallback?() } + + var flushReportingOutcomeCallCount = 0 + var flushReportingOutcomeCallback: (() throws -> Void)? + var flushReportingOutcomeReceivedCompletion: (FlushOutcomeClosure)? + func flushReportingOutcome(completion: @escaping FlushOutcomeClosure) { + flushReportingOutcomeCallCount += 1 + flushReportingOutcomeReceivedCompletion = completion + try! flushReportingOutcomeCallback?() + } } // MARK: - FeatureFlagCachingMock @@ -428,7 +437,7 @@ final class ThrottlingMock: Throttling { var runThrottledCallCount = 0 var runThrottledCallback: (() throws -> Void)? - var runThrottledReceivedRunClosure: RunClosure? + var runThrottledReceivedRunClosure: (RunClosure)? func runThrottled(_ runClosure: @escaping RunClosure) { runThrottledCallCount += 1 runThrottledReceivedRunClosure = runClosure diff --git a/LaunchDarkly/LaunchDarkly/LDClient.swift b/LaunchDarkly/LaunchDarkly/LDClient.swift index 507bd26a4..1ec13a4cb 100644 --- a/LaunchDarkly/LaunchDarkly/LDClient.swift +++ b/LaunchDarkly/LaunchDarkly/LDClient.swift @@ -263,6 +263,22 @@ public class LDClient { @objc private func didEnterBackground() { os_log("%s", log: config.logger, type: .debug, typeName(and: #function)) + // A backgrounded process is suspended as soon as it goes idle, so a delivery started here reaches the network + // only inside an activity assertion. Without one, whatever is queued waits in memory for a foreground the + // process may not live to see. + BackgroundActivity.run(reason: "LaunchDarkly event delivery") { [weak self] finished in + guard let self = self + else { + finished() + return + } + self.eventReporter.flushReportingOutcome { delivered in + if !delivered { + os_log("%s events did not reach LaunchDarkly before suspension", log: self.config.logger, type: .debug, self.typeName(and: #function)) + } + finished() + } + } Thread.performOnMain { runMode = .background } @@ -794,32 +810,6 @@ public class LDClient { plugin.register(client: self, metadata: environmentMetadata) } - /** - Tells the SDK to immediately send any currently queued events to LaunchDarkly. - - There should not normally be a need to call this function. While online, the LDClient automatically reports events - on an interval defined by `LDConfig.eventFlushInterval`. Note that this function does not block until events are - sent, it only triggers a background task to send events immediately. - */ - public func flush() { - LDClient.instancesQueue.sync(flags: .barrier) { - LDClient.instances?.forEach { $1.internalFlush() } - } - } - - private func internalFlush() { - eventReporter.flush(completion: nil) - } - - private func onEventSyncComplete(result: SynchronizingError?) { - if let synchronizingError = result { - os_log("%s result: %s", log: config.logger, type: .debug, typeName(and: #function), String(describing: synchronizingError)) - process(synchronizingError, logPrefix: typeName(and: #function)) - } else { - os_log("%s result: success", log: config.logger, type: .debug, typeName(and: #function)) - } - } - @objc private func didCloseEventSource() { os_log("%s", log: config.logger, type: .debug, typeName(and: #function)) self.connectionInformation = ConnectionInformation.lastSuccessfulConnectionCheck(connectionInformation: self.connectionInformation) @@ -1095,6 +1085,96 @@ public class LDClient { extension LDClient: TypeIdentifying { } +// MARK: - Event delivery +extension LDClient { + private func onEventSyncComplete(result: SynchronizingError?) { + if let synchronizingError = result { + os_log("%s result: %s", log: config.logger, type: .debug, typeName(and: #function), String(describing: synchronizingError)) + process(synchronizingError, logPrefix: typeName(and: #function)) + } else { + os_log("%s result: success", log: config.logger, type: .debug, typeName(and: #function)) + } + } + + /** + Tells the SDK to immediately send any currently queued events to LaunchDarkly. + + There should not normally be a need to call this function. While online, the LDClient automatically reports events + on an interval defined by `LDConfig.eventFlushInterval`. Note that this function does not block until events are + sent, it only triggers a background task to send events immediately. + */ + public func flush() { + LDClient.instancesQueue.sync(flags: .barrier) { + LDClient.instances?.forEach { $1.internalFlush() } + } + } + + /** + Sends any currently queued events to LaunchDarkly and waits up to `timeout` seconds to find out whether they got + there. + + The timeout bounds how long this call waits, not how long the delivery may run: an in-flight request is left to + finish so a payload already on the wire is not abandoned. With more than one environment, the environments share + the one budget rather than each getting a fresh copy of it. + + This is for a caller that is about to give up control — `close()`, going into the background, winding the process + down. It is not a crash-time mechanism. The SDK has no crash hook of its own on Apple, and this call does not + change that. + + Safe to call from the main thread. The wait is not free there: the watchdog terminates an application that fails + to return from a lifecycle callback in time. Keep the budget far below 15 seconds. + + - parameter timeout: How long to wait, in seconds. + - returns: Whether the pending events left the SDK's hands inside the budget. + */ + @discardableResult + public func flushAndWait(timeout: TimeInterval) -> Bool { + if timeout > LDClient.longTimeoutInterval { + os_log("%s LDClient.flushAndWait was called with a timeout greater than %f seconds. We recommend a timeout of less than %f seconds.", log: config.logger, type: .info, self.typeName(and: #function), LDClient.longTimeoutInterval, LDClient.longTimeoutInterval) + } + + let clients = LDClient.instancesQueue.sync { Array((LDClient.instances ?? [:]).values) } + let deadline = Date().addingTimeInterval(max(0, timeout)) + var delivered = true + for client in clients { + let remaining = max(0, deadline.timeIntervalSinceNow) + delivered = client.internalFlushAndWait(timeout: remaining) && delivered + } + return delivered + } + + private func internalFlush() { + eventReporter.flush(completion: nil) + } + + /// Completions that feed this wait must not hop to the main queue: lifecycle callers are already on it, and + /// waiting for a main-queue callback from the main thread is a deadlock. They also must not run on + /// `EventReporter`'s delivery queue, which is why this queue exists. + private static let flushWaitQueue = DispatchQueue(label: "com.launchdarkly.flushWait", qos: .userInitiated) + + private func internalFlushAndWait(timeout: TimeInterval) -> Bool { + let timeout = max(0, timeout) + let finished = DispatchSemaphore(value: 0) + var delivered = false + TimeoutExecutor.run( + timeout: timeout, + queue: LDClient.flushWaitQueue, + operation: { done in + self.eventReporter.flushReportingOutcome(completion: done) + }, + timeoutValue: false, + completion: { result in + delivered = result + finished.signal() + } + ) + // Slightly longer than the caller's budget so the outcome that returns is TimeoutExecutor's, not a race + // between this wait and the executor's timer. + _ = finished.wait(timeout: .now() + timeout + 0.25) + return delivered + } +} + #if DEBUG extension LDClient { func setRunMode(_ runMode: LDClientRunMode) { diff --git a/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift b/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift index 1e8a2da23..5205cd575 100644 --- a/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift +++ b/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift @@ -539,6 +539,20 @@ public final class ObjcLDClient: NSObject { ldClient.flush() } + /** + Sends any currently queued events and waits up to `timeout` for the delivery to finish. + + Returns `YES` if the events were delivered, or there were none to deliver. Returns `NO` if the timeout + expired first, or the SDK is offline, closed, or otherwise unable to deliver them. + + This is not a crash-time mechanism. Keep the budget far below 15 seconds when calling from the main thread. + + - parameter timeout: How long to wait, in seconds. + */ + @objc public func flushAndWait(timeout: TimeInterval) -> Bool { + ldClient.flushAndWait(timeout: timeout) + } + /** Starts the LDClient using the passed in `config` & `context`. Call this before requesting feature flag values. The LDClient will not go online until you call this method. Starting the LDClient means setting the `config` & `context`, setting the client online if `config.startOnline` is true (the default setting), and starting event recording. The client app must start the LDClient before it will report feature flag values. If a client does not call `start`, no methods will work. diff --git a/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift b/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift new file mode 100644 index 000000000..472166fde --- /dev/null +++ b/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift @@ -0,0 +1,46 @@ +import Foundation + +/// The extra execution time a backgrounded application can ask the system for, held for the length of one piece of +/// work. +/// +/// The system suspends a backgrounded process as soon as it goes idle, which would abandon a delivery that has only +/// just reached the network. An assertion asks the system to hold the suspension off until the work reports itself +/// finished or the system runs out of patience, whichever comes first. +/// +/// This uses `ProcessInfo.performExpiringActivity` rather than the more familiar `UIApplication.beginBackgroundTask` +/// because the framework is built extension-safe and `UIApplication.shared` is unavailable to it. Both take out the +/// same kind of assertion; they differ only in how it is asked for. +/// +/// The system may refuse the request or end it early, so this widens the window in which a delivery can finish rather +/// than guaranteeing one. Whatever does not get out is already on disk before any of this starts. +enum BackgroundActivity { + /// The longest the assertion is held for work that never reports back. The system's own budget is much shorter in + /// practice; this only exists so a caller that never calls `finished` cannot hold a thread indefinitely. + private static let maximumDuration: TimeInterval = 30 + + /// Runs `work` while holding an assertion, releasing it when `work` calls `finished`, when the system expires the + /// activity, or after `maximumDuration` — whichever comes first. + /// + /// `work` runs on a system-provided background queue and should call `finished` once; further calls do nothing. + /// On a platform with no such assertion to take, `work` runs on the calling thread and only the assertion is + /// missing. + static func run(reason: String, work: @escaping (_ finished: @escaping () -> Void) -> Void) { + #if os(iOS) || os(tvOS) || os(watchOS) + let finished = DispatchSemaphore(value: 0) + ProcessInfo.processInfo.performExpiringActivity(withReason: reason) { expired in + guard !expired + else { + // The system's second call into this block: it wants the process back, so release the first one. + finished.signal() + return + } + work { finished.signal() } + // The assertion lasts exactly as long as this block, so holding it means blocking here. This is a queue + // the system supplies for the purpose, never the main thread. + _ = finished.wait(timeout: .now() + maximumDuration) + } + #else + work {} + #endif + } +} diff --git a/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift b/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift index 06dc8e887..b10f4d0e0 100644 --- a/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift +++ b/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift @@ -2,6 +2,8 @@ import Foundation import OSLog typealias EventSyncCompleteClosure = ((SynchronizingError?) -> Void) +/// Reports whether the events a flush covered left the SDK's hands. +typealias FlushOutcomeClosure = (Bool) -> Void // sourcery: autoMockable protocol EventReporting { // sourcery: defaultMockValue = false @@ -13,6 +15,12 @@ protocol EventReporting { // swiftlint:disable:next function_parameter_count func recordFlagEvaluationEvents(flagKey: LDFlagKey, value: LDValue, defaultValue: LDValue, featureFlag: FeatureFlag?, context: LDContext, includeReason: Bool) func flush(completion: CompletionClosure?) + + /// Same as `flush`, and reports whether the pending events left the SDK's hands. + /// + /// `true` if they were delivered, refused for good, or there were none. `false` if the SDK is offline or a + /// retryable failure means they are still waiting to be sent. + func flushReportingOutcome(completion: @escaping FlushOutcomeClosure) } class NullEventReporter: EventReporting { @@ -28,6 +36,10 @@ class NullEventReporter: EventReporting { func flush(completion: CompletionClosure?) { completion?() } + + func flushReportingOutcome(completion: @escaping FlushOutcomeClosure) { + completion(true) + } } class EventReporter: EventReporting { @@ -119,6 +131,10 @@ class EventReporter: EventReporting { } func flush(completion: CompletionClosure?) { + flushReportingOutcome { _ in completion?() } + } + + func flushReportingOutcome(completion: @escaping FlushOutcomeClosure) { eventQueue.async { self.reportEvents(completion: completion) } @@ -128,12 +144,12 @@ class EventReporter: EventReporting { reportEvents(completion: nil) } - private func reportEvents(completion: CompletionClosure?) { + private func reportEvents(completion: FlushOutcomeClosure?) { guard isOnline else { os_log("%s aborted. EventReporter is offline", log: service.config.logger, type: .debug, typeName(and: #function)) reportSyncComplete(.isOffline) - completion?() + completion?(false) return } @@ -150,7 +166,7 @@ class EventReporter: EventReporting { else { os_log("%s aborted. Event store is empty", log: service.config.logger, type: .debug, typeName(and: #function)) reportSyncComplete(nil) - completion?() + completion?(true) return } @@ -166,29 +182,42 @@ class EventReporter: EventReporting { } } - private func publish(_ events: [Event], _ payloadId: String, _ completion: CompletionClosure?) { + private func publish(_ events: [Event], _ payloadId: String, _ completion: FlushOutcomeClosure?) { guard let eventData = encode(events) else { os_log("%s Failed to serialize event(s) for publication: %s", log: service.config.logger, type: .debug, typeName(and: #function), String(describing: events)) - completion?() + // Nothing is left waiting to be sent: no later attempt would produce bytes this one could not. + completion?(true) return } self.service.publishEventData(eventData, payloadId) { response in - let shouldRetry = self.processEventResponse(sentEvents: events.count, response: response.urlResponse as? HTTPURLResponse, error: response.error, isRetry: false) - if shouldRetry { + switch self.processEventResponse(sentEvents: events.count, response: response.urlResponse as? HTTPURLResponse, error: response.error, isRetry: false) { + case .settled: + completion?(true) + case .dropped: + completion?(false) + case .retryable: os_log("%s Retrying event post after delay.", log: self.service.config.logger, type: .debug, self.typeName(and: #function)) DispatchQueue.global().asyncAfter(deadline: DispatchTime.now() + 1.0) { self.service.publishEventData(eventData, payloadId) { response in - _ = self.processEventResponse(sentEvents: events.count, response: response.urlResponse as? HTTPURLResponse, error: response.error, isRetry: true) - completion?() + let outcome = self.processEventResponse(sentEvents: events.count, response: response.urlResponse as? HTTPURLResponse, error: response.error, isRetry: true) + completion?(outcome == .settled) } } - } else { - completion?() } } } + /// What a delivery attempt's response means for the events it carried. + private enum DeliveryOutcome { + /// Accepted, or refused in a way no further attempt would get past. Either way they are off our hands. + case settled + /// A failure another attempt might get past. + case retryable + /// A failure with no attempt left to make. + case dropped + } + /// Encodes a run of events as the array the events endpoint takes. /// /// One `JSONWriter` covers the whole run rather than one per event, so its byte buffer and the capacity it has @@ -212,7 +241,7 @@ class EventReporter: EventReporting { return payload } - private func processEventResponse(sentEvents: Int, response: HTTPURLResponse?, error: Error?, isRetry: Bool) -> Bool { + private func processEventResponse(sentEvents: Int, response: HTTPURLResponse?, error: Error?, isRetry: Bool) -> DeliveryOutcome { if error == nil && (200..<300).contains(response?.statusCode ?? 0) { let serverTime = response?.headerDate ?? self.lastEventResponseDate if serverTime > self.lastEventResponseDate { @@ -221,13 +250,13 @@ class EventReporter: EventReporting { os_log("%s Completed sending %d event(s)", log: service.config.logger, type: .debug, typeName(and: #function), sentEvents) self.reportSyncComplete(nil) - return false + return .settled } if let statusCode = response?.statusCode, (400..<500).contains(statusCode) && ![400, 408, 429].contains(statusCode) { os_log("%s dropping events due to non-retriable response: %s", log: service.config.logger, type: .debug, typeName(and: #function), String(describing: response)) self.reportSyncComplete(.response(response)) - return false + return .settled } os_log("%s Sending events failed with error: %s response: %s", log: service.config.logger, type: .debug, typeName(and: #function), String(describing: error), String(describing: response)) @@ -239,10 +268,10 @@ class EventReporter: EventReporting { } else { reportSyncComplete(.response(response)) } - return false + return .dropped } - return true + return .retryable } private func reportSyncComplete(_ result: SynchronizingError?) { diff --git a/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift b/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift index 42ca7482d..4f24beb01 100644 --- a/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift +++ b/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift @@ -1284,6 +1284,15 @@ final class LDClientSpec: QuickSpec { expect(testContext.eventReporterMock.isOnline) == true expect(testContext.flagSynchronizerMock.isOnline) == false } + it("tries to deliver what it has before the process is suspended") { + let testContext = TestContext(startOnline: true, enableBackgroundUpdates: false) + testContext.start() + NotificationCenter.default.post(name: SystemCapabilities.backgroundNotification!, object: self) + + // Backgrounding is the last moment the SDK is told about before the OS may suspend + // or kill the process, so it spends it trying to get the events out. + expect(testContext.eventReporterMock.flushReportingOutcomeCallCount).toEventually(equal(1)) + } it("background updates enabled") { let testContext = TestContext(startOnline: true) testContext.start() @@ -1554,6 +1563,42 @@ final class LDClientSpec: QuickSpec { expect(testContext.eventReporterMock.flushCallCount) == 1 } } + + describe("flushAndWait") { + /// Answers the reporter's completion from inside the call, which is the only chance a test gets: the + /// caller is blocked on it from the moment it returns. + func answer(_ mock: EventReportingMock, with delivered: Bool) { + mock.flushReportingOutcomeCallback = { [weak mock] in + mock?.flushReportingOutcomeReceivedCompletion?(delivered) + } + } + + it("reports true when delivery succeeds") { + let testContext = TestContext() + testContext.start() + answer(testContext.eventReporterMock, with: true) + + expect(testContext.subject.flushAndWait(timeout: 1.0)) == true + expect(testContext.eventReporterMock.flushReportingOutcomeCallCount) == 1 + } + + it("reports false when delivery cannot finish") { + let testContext = TestContext() + testContext.start() + answer(testContext.eventReporterMock, with: false) + + expect(testContext.subject.flushAndWait(timeout: 1.0)) == false + } + + it("reports false when the budget expires first") { + let testContext = TestContext() + testContext.start() + // Never answers, so the only thing that can end the wait is the timeout. + testContext.eventReporterMock.flushReportingOutcomeCallback = nil + + expect(testContext.subject.flushAndWait(timeout: 0.05)) == false + } + } } private func allFlagsSpec() { diff --git a/LaunchDarkly/LaunchDarklyTests/Mocks/ClientServiceMockFactory.swift b/LaunchDarkly/LaunchDarklyTests/Mocks/ClientServiceMockFactory.swift index d954e4fd9..f300c967a 100644 --- a/LaunchDarkly/LaunchDarklyTests/Mocks/ClientServiceMockFactory.swift +++ b/LaunchDarkly/LaunchDarklyTests/Mocks/ClientServiceMockFactory.swift @@ -76,7 +76,14 @@ final class ClientServiceMockFactory: ClientServiceCreating { makeEventReporterReceivedService = service onEventSyncComplete = onSyncComplete - return EventReportingMock() + let mock = EventReportingMock() + // Answer by default. Backgrounding holds a system activity assertion until the flush reports back, so a mock + // that only records the completion leaves that assertion held for its full timeout — which costs minutes + // across the suite. A test that wants a different answer, or none, overwrites this. + mock.flushReportingOutcomeCallback = { [weak mock] in + mock?.flushReportingOutcomeReceivedCompletion?(true) + } + return mock } func makeEventReporter(config: LDConfig, service: DarklyServiceProvider) -> EventReporting { diff --git a/SourceryTemplates/mocks.stencil b/SourceryTemplates/mocks.stencil index b03faefb8..9fa61fd20 100644 --- a/SourceryTemplates/mocks.stencil +++ b/SourceryTemplates/mocks.stencil @@ -25,7 +25,7 @@ final class {{ type.name }}Mock: {{ type.name }} { var {{ method.callName }}CallCount = 0 var {{ method.callName }}Callback: (() throws -> Void)? {% if method.throws %} var {{ method.callName }}ShouldThrow: Error?{% endif %} -{% if method.parameters.count == 1 %} var {{ method.callName }}Received{% for param in method.parameters %}{{ param.name|upperFirstLetter }}: {{ param.typeName.unwrappedTypeName }}?{% endfor %} +{% if method.parameters.count == 1 %} var {{ method.callName }}Received{% for param in method.parameters %}{{ param.name|upperFirstLetter }}: {% if param.typeAttributes.escaping %}({{ param.unwrappedTypeName }})?{% else %}{{ param.typeName.unwrappedTypeName }}?{% endif %}{% endfor %} {% else %}{% if not method.parameters.count == 0 %} var {{ method.callName }}ReceivedArguments: ({% for param in method.parameters %}{{ param.name }}: {% if param.typeAttributes.escaping %}{{ param.unwrappedTypeName }}{% else %}{{ param.typeName }}{% endif %}{% if not forloop.last %}, {% endif %}{% endfor %})?{% endif %} {% endif %} {% if not method.returnTypeName.isVoid %} var {{ method.callName }}ReturnValue: {{ method.returnTypeName }}{% if method.annotations.DefaultReturnValue %} = {{ method.annotations.DefaultReturnValue }}{% else %}{% if not method.isOptionalReturnType %}!{% endif %}{% endif %}{% endif %} From dc9d1e69e8412043948eb5a895406c8cfab17da0 Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Thu, 1 Oct 2026 09:00:55 -0700 Subject: [PATCH 2/4] fix(events): answer a bounded flush about the delivery already in flight A flush starting while a delivery was on the wire found an empty event store -- the events had left it when the request was sent -- and reported success for events that could still fail. Backgrounding is where that mattered: the hook released its activity assertion as soon as it heard success, letting the system suspend the process out from under the very request it was holding the assertion for. A delivery now claims the reporter for the length of its round trip, and a flush arriving inside that window is answered by the pass that follows it, with the in-flight outcome folded into its answer. A tier 2 failure drops the events rather than leaving them for that pass to retry, so a waiter told only about the pass would hear success for events that are gone. internalFlushAndWait read its result after a wait that had already expired, which only the semaphore's signal orders against the write. BackgroundActivity's documentation claimed undelivered events were already on disk. Nothing is written until tier 3. Co-authored-by: Cursor --- LaunchDarkly/LaunchDarkly/LDClient.swift | 5 +- .../ServiceObjects/BackgroundActivity.swift | 2 +- .../ServiceObjects/EventReporter.swift | 53 +++++++++++- .../Mocks/DarklyServiceMock.swift | 17 ++++ .../ServiceObjects/EventReporterSpec.swift | 80 +++++++++++++++++++ 5 files changed, 153 insertions(+), 4 deletions(-) diff --git a/LaunchDarkly/LaunchDarkly/LDClient.swift b/LaunchDarkly/LaunchDarkly/LDClient.swift index 842429783..2243c9dee 100644 --- a/LaunchDarkly/LaunchDarkly/LDClient.swift +++ b/LaunchDarkly/LaunchDarkly/LDClient.swift @@ -1170,8 +1170,9 @@ extension LDClient { ) // Slightly longer than the caller's budget so the outcome that returns is TimeoutExecutor's, not a race // between this wait and the executor's timer. - _ = finished.wait(timeout: .now() + timeout + 0.25) - return delivered + let answered = finished.wait(timeout: .now() + timeout + 0.25) + // Only the signal orders that write against this read, so a wait that expired first must not look at it. + return answered == .success && delivered } } diff --git a/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift b/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift index 472166fde..dccdcab26 100644 --- a/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift +++ b/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift @@ -12,7 +12,7 @@ import Foundation /// same kind of assertion; they differ only in how it is asked for. /// /// The system may refuse the request or end it early, so this widens the window in which a delivery can finish rather -/// than guaranteeing one. Whatever does not get out is already on disk before any of this starts. +/// than guaranteeing one. What does not get out stays queued for the next delivery, if the process lives to make one. enum BackgroundActivity { /// The longest the assertion is held for work that never reports back. The system's own budget is much shorter in /// practice; this only exists so a caller that never calls `finished` cannot hold a thread indefinitely. diff --git a/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift b/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift index 65498610a..b14d9966b 100644 --- a/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift +++ b/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift @@ -57,6 +57,19 @@ class EventReporter: EventReporting { private(set) var eventStore: [Event] = [] private(set) var contextSummarizer: ContextSummarizer + /// Whether a delivery is running, from the request being sent until its response has been handled. + /// + /// `eventQueue` serializes the *start* of a delivery but not the round trip it waits on, and the events it carries + /// have already left `eventStore`. Without this, a flush beginning inside that window would find nothing left to + /// send and report success while the events it was asked about were still on the wire. + private var isDelivering = false + + /// Whether a delivery was asked for while one was already running. + private var hasWaitingRequest = false + + /// Callers waiting on the pass that `hasWaitingRequest` will start. + private var waitingCompletions: [FlushOutcomeClosure] = [] + private var timerQueue = DispatchQueue(label: "com.launchdarkly.EventReporter.timerQueue") private var eventReportTimer: TimeResponding? var isReportingActive: Bool { eventReportTimer != nil } @@ -141,6 +154,17 @@ class EventReporter: EventReporting { } private func reportEvents(completion: FlushOutcomeClosure?) { + guard !isDelivering + else { + // Answered by the pass that runs once the current delivery finishes, so a caller hears about the events + // on the wire as well as anything recorded since. Starting a second delivery now would not reach them. + hasWaitingRequest = true + if let completion { + waitingCompletions.append(completion) + } + return + } + guard isOnline else { os_log("%s aborted. EventReporter is offline", log: service.config.logger, type: .debug, typeName(and: #function)) @@ -173,8 +197,35 @@ class EventReporter: EventReporting { service.diagnosticCache?.recordEventsInLastBatch(eventsInLastBatch: toPublish.count) + isDelivering = true DispatchQueue.global().async { - self.publish(toPublish, UUID().uuidString, completion) + self.publish(toPublish, UUID().uuidString) { delivered in + // `publish` reports from whichever queue the response arrived on. + self.eventQueue.async { + self.finishDelivery(delivered, completion) + } + } + } + } + + /// Releases the in-flight claim and, if a delivery was asked for while it was held, makes the one pass that covers + /// every caller that waited. + /// + /// A waiter is told what happened to this delivery as well as to that pass. Its own events may be in either, and + /// events this delivery failed to place are gone rather than left for the pass to retry, so a waiter that heard + /// only the pass's answer would be told everything went out when half of it did not. + private func finishDelivery(_ delivered: Bool, _ completion: FlushOutcomeClosure?) { + isDelivering = false + completion?(delivered) + + guard hasWaitingRequest + else { return } + + let waiting = waitingCompletions + hasWaitingRequest = false + waitingCompletions = [] + reportEvents { result in + waiting.forEach { $0(delivered && result) } } } diff --git a/LaunchDarkly/LaunchDarklyTests/Mocks/DarklyServiceMock.swift b/LaunchDarkly/LaunchDarklyTests/Mocks/DarklyServiceMock.swift index e1e36ee4e..60c75aad4 100644 --- a/LaunchDarkly/LaunchDarklyTests/Mocks/DarklyServiceMock.swift +++ b/LaunchDarkly/LaunchDarklyTests/Mocks/DarklyServiceMock.swift @@ -136,12 +136,29 @@ final class DarklyServiceMock: DarklyServiceProvider { var stubbedEventResponse: ServiceResponse? var publishEventDataCallCount = 0 var publishedEventData: Data? + /// Holds responses back instead of answering inline, so a test can act while a delivery is in flight. + var holdsEventCompletions = false + private var heldEventCompletions: [ServiceCompletionHandler] = [] func publishEventData(_ eventData: Data, _ payloadId: String, completion: ServiceCompletionHandler?) { publishEventDataCallCount += 1 publishedEventData = eventData + guard !holdsEventCompletions + else { + if let completion { + heldEventCompletions.append(completion) + } + return + } completion?(stubbedEventResponse ?? (nil, nil, nil, nil)) } + /// Answers every held request, as the network coming back would. + func releaseHeldEventCompletions() { + let held = heldEventCompletions + heldEventCompletions = [] + held.forEach { $0(stubbedEventResponse ?? (nil, nil, nil, nil)) } + } + var stubbedDiagnosticResponse: ServiceResponse? var publishDiagnosticCallCount = 0 var publishedDiagnostic: DiagnosticEvent? diff --git a/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift index a2b6bcbe2..83ff4c490 100644 --- a/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift +++ b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift @@ -68,10 +68,90 @@ final class EventReporterSpec: QuickSpec { recordEventSpec() testRecordFlagEvaluationEvents() reportEventsSpec() + flushReportingOutcomeSpec() unserializableEventSpec() reportTimerSpec() } + private func flushReportingOutcomeSpec() { + describe("flushReportingOutcome") { + var testContext: TestContext! + afterEach { + testContext.eventReporter.isOnline = false + } + + it("reports true when LaunchDarkly accepts the events") { + testContext = TestContext() + testContext.recordEvents(1) + testContext.eventReporter.isOnline = true + + var delivered: Bool? + waitUntil { done in + testContext.eventReporter.flushReportingOutcome { result in + delivered = result + done() + } + } + expect(delivered) == true + } + + it("reports true when there is nothing to deliver") { + testContext = TestContext() + testContext.eventReporter.isOnline = true + + var delivered: Bool? + waitUntil { done in + testContext.eventReporter.flushReportingOutcome { result in + delivered = result + done() + } + } + expect(delivered) == true + } + + it("reports false while offline") { + testContext = TestContext() + testContext.recordEvents(1) + + var delivered: Bool? + waitUntil { done in + testContext.eventReporter.flushReportingOutcome { result in + delivered = result + done() + } + } + expect(delivered) == false + expect(testContext.serviceMock.publishEventDataCallCount) == 0 + } + + it("waits for a delivery already in flight rather than reporting an empty store as success") { + testContext = TestContext(stubResponseSuccess: false) + testContext.recordEvents(1) + testContext.eventReporter.isOnline = true + // The response is held back, so the first delivery stays in flight with the event store already + // emptied -- the window in which a second flush would find nothing left and call that success. + testContext.serviceMock.holdsEventCompletions = true + + testContext.eventReporter.flushReportingOutcome { _ in } + expect(testContext.serviceMock.publishEventDataCallCount).toEventually(equal(1)) + + var delivered: Bool? + testContext.eventReporter.flushReportingOutcome { result in delivered = result } + // Nothing is answered while the events this caller asked about are still on the wire. + Thread.sleep(forTimeInterval: 0.2) + expect(delivered).to(beNil()) + + testContext.serviceMock.holdsEventCompletions = false + testContext.serviceMock.releaseHeldEventCompletions() + + // The delivery failed, and a tier 2 failure drops the events rather than leaving them for a later + // attempt, so the honest answer is that they did not get out. Long enough for the retry the first + // failure schedules, which is what the caller is really waiting on. + expect(delivered).toEventually(beFalse(), timeout: .seconds(10)) + } + } + } + private func initSpec() { describe("init") { var testContext: TestContext! From 6c3b8f9ee309270144e1be251758b448f8a5cbbe Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Thu, 1 Oct 2026 09:20:59 -0700 Subject: [PATCH 3/4] shorten comments --- LaunchDarkly/LaunchDarkly/LDClient.swift | 31 +++++---------- .../ServiceObjects/BackgroundActivity.swift | 31 ++++----------- .../ServiceObjects/EventReporter.swift | 38 ++++++------------- .../LaunchDarklyTests/LDClientSpec.swift | 6 +-- .../Mocks/ClientServiceMockFactory.swift | 4 +- .../Mocks/DarklyServiceMock.swift | 4 +- .../ServiceObjects/EventReporterSpec.swift | 8 +--- 7 files changed, 35 insertions(+), 87 deletions(-) diff --git a/LaunchDarkly/LaunchDarkly/LDClient.swift b/LaunchDarkly/LaunchDarkly/LDClient.swift index 2243c9dee..eb420d8bf 100644 --- a/LaunchDarkly/LaunchDarkly/LDClient.swift +++ b/LaunchDarkly/LaunchDarkly/LDClient.swift @@ -263,9 +263,7 @@ public class LDClient { @objc private func didEnterBackground() { os_log("%s", log: config.logger, type: .debug, typeName(and: #function)) - // A backgrounded process is suspended as soon as it goes idle, so a delivery started here reaches the network - // only inside an activity assertion. Without one, whatever is queued waits in memory for a foreground the - // process may not live to see. + // A backgrounded process is suspended once idle, so deliver inside an activity assertion. BackgroundActivity.run(reason: "LaunchDarkly event delivery") { [weak self] finished in guard let self = self else { @@ -1110,22 +1108,16 @@ extension LDClient { } /** - Sends any currently queued events to LaunchDarkly and waits up to `timeout` seconds to find out whether they got - there. + Sends any currently queued events to LaunchDarkly and waits up to `timeout` seconds for the result. - The timeout bounds how long this call waits, not how long the delivery may run: an in-flight request is left to - finish so a payload already on the wire is not abandoned. With more than one environment, the environments share - the one budget rather than each getting a fresh copy of it. + The timeout bounds the wait, not the delivery: an in-flight request is left to finish. Multiple environments + share one budget. - This is for a caller that is about to give up control — `close()`, going into the background, winding the process - down. It is not a crash-time mechanism. The SDK has no crash hook of its own on Apple, and this call does not - change that. - - Safe to call from the main thread. The wait is not free there: the watchdog terminates an application that fails - to return from a lifecycle callback in time. Keep the budget far below 15 seconds. + This is not a crash-time mechanism. It is safe to call from the main thread, but keep the budget well below + 15 seconds there. - parameter timeout: How long to wait, in seconds. - - returns: Whether the pending events left the SDK's hands inside the budget. + - returns: Whether the pending events left the SDK's hands within the budget. */ @discardableResult public func flushAndWait(timeout: TimeInterval) -> Bool { @@ -1147,9 +1139,7 @@ extension LDClient { eventReporter.flush(completion: nil) } - /// Completions that feed this wait must not hop to the main queue: lifecycle callers are already on it, and - /// waiting for a main-queue callback from the main thread is a deadlock. They also must not run on - /// `EventReporter`'s delivery queue, which is why this queue exists. + /// Neither main (callers may be blocked on it) nor the reporter's queue, so the wait cannot deadlock. private static let flushWaitQueue = DispatchQueue(label: "com.launchdarkly.flushWait", qos: .userInitiated) private func internalFlushAndWait(timeout: TimeInterval) -> Bool { @@ -1168,10 +1158,9 @@ extension LDClient { finished.signal() } ) - // Slightly longer than the caller's budget so the outcome that returns is TimeoutExecutor's, not a race - // between this wait and the executor's timer. + // A little past the budget, so TimeoutExecutor's timer decides the outcome. let answered = finished.wait(timeout: .now() + timeout + 0.25) - // Only the signal orders that write against this read, so a wait that expired first must not look at it. + // `delivered` is only safe to read once the semaphore was signaled. return answered == .success && delivered } } diff --git a/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift b/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift index dccdcab26..101273954 100644 --- a/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift +++ b/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift @@ -1,42 +1,27 @@ import Foundation -/// The extra execution time a backgrounded application can ask the system for, held for the length of one piece of -/// work. +/// Holds a background activity assertion so a backgrounded process is not suspended mid-delivery. /// -/// The system suspends a backgrounded process as soon as it goes idle, which would abandon a delivery that has only -/// just reached the network. An assertion asks the system to hold the suspension off until the work reports itself -/// finished or the system runs out of patience, whichever comes first. -/// -/// This uses `ProcessInfo.performExpiringActivity` rather than the more familiar `UIApplication.beginBackgroundTask` -/// because the framework is built extension-safe and `UIApplication.shared` is unavailable to it. Both take out the -/// same kind of assertion; they differ only in how it is asked for. -/// -/// The system may refuse the request or end it early, so this widens the window in which a delivery can finish rather -/// than guaranteeing one. What does not get out stays queued for the next delivery, if the process lives to make one. +/// Uses `ProcessInfo.performExpiringActivity` because the framework is extension-safe and cannot use +/// `UIApplication.beginBackgroundTask`. The system may refuse or end the assertion early. enum BackgroundActivity { - /// The longest the assertion is held for work that never reports back. The system's own budget is much shorter in - /// practice; this only exists so a caller that never calls `finished` cannot hold a thread indefinitely. + /// Upper bound for work that never calls `finished`. private static let maximumDuration: TimeInterval = 30 - /// Runs `work` while holding an assertion, releasing it when `work` calls `finished`, when the system expires the - /// activity, or after `maximumDuration` — whichever comes first. - /// - /// `work` runs on a system-provided background queue and should call `finished` once; further calls do nothing. - /// On a platform with no such assertion to take, `work` runs on the calling thread and only the assertion is - /// missing. + /// Runs `work` under an assertion released when `work` calls `finished`, the system expires it, or + /// `maximumDuration` passes. Without assertions on this platform, `work` runs on the calling thread. static func run(reason: String, work: @escaping (_ finished: @escaping () -> Void) -> Void) { #if os(iOS) || os(tvOS) || os(watchOS) let finished = DispatchSemaphore(value: 0) ProcessInfo.processInfo.performExpiringActivity(withReason: reason) { expired in guard !expired else { - // The system's second call into this block: it wants the process back, so release the first one. + // Expiration: release the blocked first call. finished.signal() return } work { finished.signal() } - // The assertion lasts exactly as long as this block, so holding it means blocking here. This is a queue - // the system supplies for the purpose, never the main thread. + // The assertion lasts as long as this block, which runs on a system queue, not main. _ = finished.wait(timeout: .now() + maximumDuration) } #else diff --git a/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift b/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift index b14d9966b..887d68d6f 100644 --- a/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift +++ b/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift @@ -16,10 +16,8 @@ protocol EventReporting { func recordFlagEvaluationEvents(flagKey: LDFlagKey, value: LDValue, defaultValue: LDValue, featureFlag: FeatureFlag?, context: LDContext, includeReason: Bool) func flush(completion: CompletionClosure?) - /// Same as `flush`, and reports whether the pending events left the SDK's hands. - /// - /// `true` if they were delivered, refused for good, or there were none. `false` if the SDK is offline or a - /// retryable failure means they are still waiting to be sent. + /// Like `flush`. Reports `true` if events were delivered, permanently refused, or absent; `false` if offline or + /// delivery failed. func flushReportingOutcome(completion: @escaping FlushOutcomeClosure) } @@ -57,17 +55,11 @@ class EventReporter: EventReporting { private(set) var eventStore: [Event] = [] private(set) var contextSummarizer: ContextSummarizer - /// Whether a delivery is running, from the request being sent until its response has been handled. - /// - /// `eventQueue` serializes the *start* of a delivery but not the round trip it waits on, and the events it carries - /// have already left `eventStore`. Without this, a flush beginning inside that window would find nothing left to - /// send and report success while the events it was asked about were still on the wire. + /// True from sending a request until its response is handled. Its events have already left `eventStore`. private var isDelivering = false - - /// Whether a delivery was asked for while one was already running. + /// A delivery was requested while one was in flight. private var hasWaitingRequest = false - - /// Callers waiting on the pass that `hasWaitingRequest` will start. + /// Callers answered by the pass after the in-flight delivery. private var waitingCompletions: [FlushOutcomeClosure] = [] private var timerQueue = DispatchQueue(label: "com.launchdarkly.EventReporter.timerQueue") @@ -156,8 +148,7 @@ class EventReporter: EventReporting { private func reportEvents(completion: FlushOutcomeClosure?) { guard !isDelivering else { - // Answered by the pass that runs once the current delivery finishes, so a caller hears about the events - // on the wire as well as anything recorded since. Starting a second delivery now would not reach them. + // Answered after the in-flight delivery, which may hold this caller's events. hasWaitingRequest = true if let completion { waitingCompletions.append(completion) @@ -200,7 +191,6 @@ class EventReporter: EventReporting { isDelivering = true DispatchQueue.global().async { self.publish(toPublish, UUID().uuidString) { delivered in - // `publish` reports from whichever queue the response arrived on. self.eventQueue.async { self.finishDelivery(delivered, completion) } @@ -208,12 +198,8 @@ class EventReporter: EventReporting { } } - /// Releases the in-flight claim and, if a delivery was asked for while it was held, makes the one pass that covers - /// every caller that waited. - /// - /// A waiter is told what happened to this delivery as well as to that pass. Its own events may be in either, and - /// events this delivery failed to place are gone rather than left for the pass to retry, so a waiter that heard - /// only the pass's answer would be told everything went out when half of it did not. + /// Ends the in-flight delivery and runs one pass for any waiters. Failed events are dropped, not retried, so a + /// waiter succeeds only if both this delivery and that pass did. private func finishDelivery(_ delivered: Bool, _ completion: FlushOutcomeClosure?) { isDelivering = false completion?(delivered) @@ -233,7 +219,7 @@ class EventReporter: EventReporting { guard let eventData = encode(events) else { os_log("%s Failed to serialize event(s) for publication: %s", log: service.config.logger, type: .error, typeName(and: #function), String(describing: events)) - // Nothing is left waiting to be sent: no later attempt would produce bytes this one could not. + // Encoding is deterministic, so no retry would succeed. completion?(true) return } @@ -255,13 +241,11 @@ class EventReporter: EventReporting { } } - /// What a delivery attempt's response means for the events it carried. private enum DeliveryOutcome { - /// Accepted, or refused in a way no further attempt would get past. Either way they are off our hands. + /// Accepted or permanently refused. case settled - /// A failure another attempt might get past. case retryable - /// A failure with no attempt left to make. + /// Failed with no retry left. case dropped } diff --git a/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift b/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift index 1d0352d74..63ac6e60b 100644 --- a/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift +++ b/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift @@ -1365,8 +1365,6 @@ final class LDClientSpec: QuickSpec { testContext.start() NotificationCenter.default.post(name: SystemCapabilities.backgroundNotification!, object: self) - // Backgrounding is the last moment the SDK is told about before the OS may suspend - // or kill the process, so it spends it trying to get the events out. expect(testContext.eventReporterMock.flushReportingOutcomeCallCount).toEventually(equal(1)) } it("background updates enabled") { @@ -1641,8 +1639,7 @@ final class LDClientSpec: QuickSpec { } describe("flushAndWait") { - /// Answers the reporter's completion from inside the call, which is the only chance a test gets: the - /// caller is blocked on it from the moment it returns. + /// Answers inside the call, because `flushAndWait` blocks the test thread. func answer(_ mock: EventReportingMock, with delivered: Bool) { mock.flushReportingOutcomeCallback = { [weak mock] in mock?.flushReportingOutcomeReceivedCompletion?(delivered) @@ -1669,7 +1666,6 @@ final class LDClientSpec: QuickSpec { it("reports false when the budget expires first") { let testContext = TestContext() testContext.start() - // Never answers, so the only thing that can end the wait is the timeout. testContext.eventReporterMock.flushReportingOutcomeCallback = nil expect(testContext.subject.flushAndWait(timeout: 0.05)) == false diff --git a/LaunchDarkly/LaunchDarklyTests/Mocks/ClientServiceMockFactory.swift b/LaunchDarkly/LaunchDarklyTests/Mocks/ClientServiceMockFactory.swift index f300c967a..ff4f264cf 100644 --- a/LaunchDarkly/LaunchDarklyTests/Mocks/ClientServiceMockFactory.swift +++ b/LaunchDarkly/LaunchDarklyTests/Mocks/ClientServiceMockFactory.swift @@ -77,9 +77,7 @@ final class ClientServiceMockFactory: ClientServiceCreating { onEventSyncComplete = onSyncComplete let mock = EventReportingMock() - // Answer by default. Backgrounding holds a system activity assertion until the flush reports back, so a mock - // that only records the completion leaves that assertion held for its full timeout — which costs minutes - // across the suite. A test that wants a different answer, or none, overwrites this. + // Answer by default so backgrounding does not hold its activity assertion until timeout. mock.flushReportingOutcomeCallback = { [weak mock] in mock?.flushReportingOutcomeReceivedCompletion?(true) } diff --git a/LaunchDarkly/LaunchDarklyTests/Mocks/DarklyServiceMock.swift b/LaunchDarkly/LaunchDarklyTests/Mocks/DarklyServiceMock.swift index 60c75aad4..12545b74a 100644 --- a/LaunchDarkly/LaunchDarklyTests/Mocks/DarklyServiceMock.swift +++ b/LaunchDarkly/LaunchDarklyTests/Mocks/DarklyServiceMock.swift @@ -136,7 +136,7 @@ final class DarklyServiceMock: DarklyServiceProvider { var stubbedEventResponse: ServiceResponse? var publishEventDataCallCount = 0 var publishedEventData: Data? - /// Holds responses back instead of answering inline, so a test can act while a delivery is in flight. + /// Holds responses so a test can act while a delivery is in flight. var holdsEventCompletions = false private var heldEventCompletions: [ServiceCompletionHandler] = [] func publishEventData(_ eventData: Data, _ payloadId: String, completion: ServiceCompletionHandler?) { @@ -152,7 +152,7 @@ final class DarklyServiceMock: DarklyServiceProvider { completion?(stubbedEventResponse ?? (nil, nil, nil, nil)) } - /// Answers every held request, as the network coming back would. + /// Answers every held request. func releaseHeldEventCompletions() { let held = heldEventCompletions heldEventCompletions = [] diff --git a/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift index 83ff4c490..622dd0f5c 100644 --- a/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift +++ b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift @@ -128,8 +128,7 @@ final class EventReporterSpec: QuickSpec { testContext = TestContext(stubResponseSuccess: false) testContext.recordEvents(1) testContext.eventReporter.isOnline = true - // The response is held back, so the first delivery stays in flight with the event store already - // emptied -- the window in which a second flush would find nothing left and call that success. + // Keep the first delivery in flight with the event store already empty. testContext.serviceMock.holdsEventCompletions = true testContext.eventReporter.flushReportingOutcome { _ in } @@ -137,16 +136,13 @@ final class EventReporterSpec: QuickSpec { var delivered: Bool? testContext.eventReporter.flushReportingOutcome { result in delivered = result } - // Nothing is answered while the events this caller asked about are still on the wire. Thread.sleep(forTimeInterval: 0.2) expect(delivered).to(beNil()) testContext.serviceMock.holdsEventCompletions = false testContext.serviceMock.releaseHeldEventCompletions() - // The delivery failed, and a tier 2 failure drops the events rather than leaving them for a later - // attempt, so the honest answer is that they did not get out. Long enough for the retry the first - // failure schedules, which is what the caller is really waiting on. + // Allows for the one-second retry; the failed events are dropped. expect(delivered).toEventually(beFalse(), timeout: .seconds(10)) } } From 57f04c3698e6667bbeb738916fcc9f5482d6e38d Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Thu, 1 Oct 2026 13:35:38 -0700 Subject: [PATCH 4/4] fix(events): report a closed client and a refused in-flight delivery correctly from flushAndWait Co-authored-by: Cursor --- LaunchDarkly/LaunchDarkly/LDClient.swift | 6 ++++- .../ServiceObjects/EventReporter.swift | 4 ++- .../LaunchDarklyTests/LDClientSpec.swift | 9 +++++++ .../ServiceObjects/EventReporterSpec.swift | 25 +++++++++++++++++++ 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/LaunchDarkly/LaunchDarkly/LDClient.swift b/LaunchDarkly/LaunchDarkly/LDClient.swift index eb420d8bf..1a0368c2a 100644 --- a/LaunchDarkly/LaunchDarkly/LDClient.swift +++ b/LaunchDarkly/LaunchDarkly/LDClient.swift @@ -1125,7 +1125,11 @@ extension LDClient { os_log("%s LDClient.flushAndWait was called with a timeout greater than %f seconds. We recommend a timeout of less than %f seconds.", log: config.logger, type: .info, self.typeName(and: #function), LDClient.longTimeoutInterval, LDClient.longTimeoutInterval) } - let clients = LDClient.instancesQueue.sync { Array((LDClient.instances ?? [:]).values) } + guard let clients = LDClient.instancesQueue.sync(execute: { LDClient.instances.map { Array($0.values) } }) + else { + os_log("%s called on a closed client", log: config.logger, type: .debug, self.typeName(and: #function)) + return false + } let deadline = Date().addingTimeInterval(max(0, timeout)) var delivered = true for client in clients { diff --git a/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift b/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift index 887d68d6f..1c2645e41 100644 --- a/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift +++ b/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift @@ -210,8 +210,10 @@ class EventReporter: EventReporting { let waiting = waitingCompletions hasWaitingRequest = false waitingCompletions = [] + // A terminal response takes the reporter offline after settling; with nothing left, that is not a failure. + let nothingPending = eventStore.isEmpty && !contextSummarizer.hasLoggedRequests reportEvents { result in - waiting.forEach { $0(delivered && result) } + waiting.forEach { $0(delivered && (result || nothingPending)) } } } diff --git a/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift b/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift index 63ac6e60b..e226f27cb 100644 --- a/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift +++ b/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift @@ -1670,6 +1670,15 @@ final class LDClientSpec: QuickSpec { expect(testContext.subject.flushAndWait(timeout: 0.05)) == false } + + it("reports false once closed") { + let testContext = TestContext() + testContext.start() + answer(testContext.eventReporterMock, with: true) + testContext.subject.close() + + expect(testContext.subject.flushAndWait(timeout: 1.0)) == false + } } } diff --git a/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift index 622dd0f5c..3f8b74167 100644 --- a/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift +++ b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift @@ -145,6 +145,31 @@ final class EventReporterSpec: QuickSpec { // Allows for the one-second retry; the failed events are dropped. expect(delivered).toEventually(beFalse(), timeout: .seconds(10)) } + + it("reports true to a waiter when the in-flight delivery is permanently refused") { + testContext = TestContext() + let unauthorized = HTTPURLResponse(url: testContext.serviceMock.config.eventsUrl, + statusCode: HTTPURLResponse.StatusCodes.unauthorized, + httpVersion: DarklyServiceMock.Constants.httpVersion, + headerFields: nil) + testContext.serviceMock.stubbedEventResponse = (nil, unauthorized, nil, nil) + testContext.recordEvents(1) + testContext.eventReporter.isOnline = true + testContext.serviceMock.holdsEventCompletions = true + + testContext.eventReporter.flushReportingOutcome { _ in } + expect(testContext.serviceMock.publishEventDataCallCount).toEventually(equal(1)) + + var delivered: Bool? + testContext.eventReporter.flushReportingOutcome { result in delivered = result } + Thread.sleep(forTimeInterval: 0.2) + + testContext.serviceMock.holdsEventCompletions = false + testContext.serviceMock.releaseHeldEventCompletions() + + expect(delivered).toEventually(beTrue()) + expect(testContext.eventReporter.isOnline) == false + } } }