From 05b56287f67f241f4f620e55c3a831a2f7872c8e Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Fri, 18 Sep 2026 18:29:12 -0700 Subject: [PATCH 01/11] 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 02/11] 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 03/11] 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 04/11] 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 + } } } From f11a86ccd1964bcbd7978ae7421bfcbe2e824bfe Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Wed, 7 Oct 2026 15:17:41 -0700 Subject: [PATCH 05/11] refactor(events): Name the queued delivery a pending flush and move flush into LDClientFlush.swift Co-authored-by: Cursor --- LaunchDarkly.xcodeproj/project.pbxproj | 10 +++ LaunchDarkly/LaunchDarkly/LDClient.swift | 85 ++----------------- LaunchDarkly/LaunchDarkly/LDClientFlush.swift | 79 +++++++++++++++++ .../ServiceObjects/EventReporter.swift | 30 ++++--- .../ServiceObjects/EventReporterSpec.swift | 2 +- 5 files changed, 111 insertions(+), 95 deletions(-) create mode 100644 LaunchDarkly/LaunchDarkly/LDClientFlush.swift diff --git a/LaunchDarkly.xcodeproj/project.pbxproj b/LaunchDarkly.xcodeproj/project.pbxproj index 2df0d179f..16fb0ed2b 100644 --- a/LaunchDarkly.xcodeproj/project.pbxproj +++ b/LaunchDarkly.xcodeproj/project.pbxproj @@ -278,9 +278,13 @@ A350386F2F4F4F610032BA9F /* IdentifySeriesContext.swift in Sources */ = {isa = PBXBuildFile; fileRef = A350386C2F4F4F610032BA9F /* IdentifySeriesContext.swift */; }; A35038702F4F4F610032BA9F /* IdentifySeriesContext.swift in Sources */ = {isa = PBXBuildFile; fileRef = A350386C2F4F4F610032BA9F /* IdentifySeriesContext.swift */; }; A35038772F4F58660032BA9F /* LDClientIdentifyHook.swift in Sources */ = {isa = PBXBuildFile; fileRef = A35038762F4F58660032BA9F /* LDClientIdentifyHook.swift */; }; + 99686398CCB7515A18247662 /* LDClientFlush.swift in Sources */ = {isa = PBXBuildFile; fileRef = 86CB57159BC81898DE96A352 /* LDClientFlush.swift */; }; A35038782F4F58660032BA9F /* LDClientIdentifyHook.swift in Sources */ = {isa = PBXBuildFile; fileRef = A35038762F4F58660032BA9F /* LDClientIdentifyHook.swift */; }; + B04B36074760CCA9F48E1851 /* LDClientFlush.swift in Sources */ = {isa = PBXBuildFile; fileRef = 86CB57159BC81898DE96A352 /* LDClientFlush.swift */; }; A35038792F4F58660032BA9F /* LDClientIdentifyHook.swift in Sources */ = {isa = PBXBuildFile; fileRef = A35038762F4F58660032BA9F /* LDClientIdentifyHook.swift */; }; + 121B6A1A325C9A7D5EEA2EAB /* LDClientFlush.swift in Sources */ = {isa = PBXBuildFile; fileRef = 86CB57159BC81898DE96A352 /* LDClientFlush.swift */; }; A350387A2F4F58660032BA9F /* LDClientIdentifyHook.swift in Sources */ = {isa = PBXBuildFile; fileRef = A35038762F4F58660032BA9F /* LDClientIdentifyHook.swift */; }; + 43DC6AA5DB0BE9A7B697C10F /* LDClientFlush.swift in Sources */ = {isa = PBXBuildFile; fileRef = 86CB57159BC81898DE96A352 /* LDClientFlush.swift */; }; A350387C2F4F5A380032BA9F /* Data+Compression.swift in Sources */ = {isa = PBXBuildFile; fileRef = A350387B2F4F5A380032BA9F /* Data+Compression.swift */; }; A350387D2F4F5A380032BA9F /* Data+Compression.swift in Sources */ = {isa = PBXBuildFile; fileRef = A350387B2F4F5A380032BA9F /* Data+Compression.swift */; }; A350387E2F4F5A380032BA9F /* Data+Compression.swift in Sources */ = {isa = PBXBuildFile; fileRef = A350387B2F4F5A380032BA9F /* Data+Compression.swift */; }; @@ -597,6 +601,7 @@ A3470C362B7C1ACE00951CEE /* LDValueDecoder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LDValueDecoder.swift; sourceTree = ""; }; A350386C2F4F4F610032BA9F /* IdentifySeriesContext.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IdentifySeriesContext.swift; sourceTree = ""; }; A35038762F4F58660032BA9F /* LDClientIdentifyHook.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LDClientIdentifyHook.swift; sourceTree = ""; }; + 86CB57159BC81898DE96A352 /* LDClientFlush.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LDClientFlush.swift; sourceTree = ""; }; A350387B2F4F5A380032BA9F /* Data+Compression.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Data+Compression.swift"; sourceTree = ""; }; A35038802F4F965E0032BA9F /* LDClientIdentifyHookSpec.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LDClientIdentifyHookSpec.swift; sourceTree = ""; }; A35038822F4F96680032BA9F /* LDClientEvaluationHookSpec.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LDClientEvaluationHookSpec.swift; sourceTree = ""; }; @@ -809,6 +814,7 @@ isa = PBXGroup; children = ( A35038762F4F58660032BA9F /* LDClientIdentifyHook.swift */, + 86CB57159BC81898DE96A352 /* LDClientFlush.swift */, A380B0982B60178D00AB64A6 /* PrivacyInfo.xcprivacy */, 83B6C4B51F4DE7630055351C /* LDCommon.swift */, 8354EFDC1F26380700C05156 /* LDClient.swift */, @@ -1485,6 +1491,7 @@ C43C37E8238DF22D003C1624 /* LDEvaluationDetail.swift in Sources */, 8311884C2113ADDE00D77CB5 /* FlagChangeObserver.swift in Sources */, A35038772F4F58660032BA9F /* LDClientIdentifyHook.swift in Sources */, + 99686398CCB7515A18247662 /* LDClientFlush.swift in Sources */, A350387C2F4F5A380032BA9F /* Data+Compression.swift in Sources */, A3BA7CF62BD05A280000DB28 /* EvaluationSeriesContext.swift in Sources */, A3470C3A2B7C1ACE00951CEE /* LDValueDecoder.swift in Sources */, @@ -1621,6 +1628,7 @@ A3A8BCD42B7EAA89009A77E4 /* SheddingQueue.swift in Sources */, B4C9D43A2489E20A004A9B03 /* DiagnosticReporter.swift in Sources */, A35038792F4F58660032BA9F /* LDClientIdentifyHook.swift in Sources */, + 121B6A1A325C9A7D5EEA2EAB /* LDClientFlush.swift in Sources */, 831EF36A20655E730001C643 /* ObjcLDChangedFlag.swift in Sources */, 50EE85C22EA0487F007CC662 /* TimeoutExecutor.swift in Sources */, DAF5695D58E28B8CF9221213 /* LDContextJSONWriter.swift in Sources */, @@ -1668,6 +1676,7 @@ 8358F25E1F474E5900ECE1AF /* LDChangedFlag.swift in Sources */, 83D559741FD87CC9002D10C8 /* KeyedValueCache.swift in Sources */, A35038782F4F58660032BA9F /* LDClientIdentifyHook.swift in Sources */, + B04B36074760CCA9F48E1851 /* LDClientFlush.swift in Sources */, A350387D2F4F5A380032BA9F /* Data+Compression.swift in Sources */, A3BA7CF32BD05A280000DB28 /* EvaluationSeriesContext.swift in Sources */, A3470C372B7C1ACE00951CEE /* LDValueDecoder.swift in Sources */, @@ -1823,6 +1832,7 @@ A358D6F02A4DE9EB00270C60 /* WatchOSEnvironmentReporter.swift in Sources */, 831AAE2D20A9E4F600B46DBA /* Throttler.swift in Sources */, A350387A2F4F58660032BA9F /* LDClientIdentifyHook.swift in Sources */, + 43DC6AA5DB0BE9A7B697C10F /* LDClientFlush.swift in Sources */, A350387E2F4F5A380032BA9F /* Data+Compression.swift in Sources */, A3BA7CF42BD05A280000DB28 /* EvaluationSeriesContext.swift in Sources */, A3470C382B7C1ACE00951CEE /* LDValueDecoder.swift in Sources */, diff --git a/LaunchDarkly/LaunchDarkly/LDClient.swift b/LaunchDarkly/LaunchDarkly/LDClient.swift index 1a0368c2a..4fbe8c0e0 100644 --- a/LaunchDarkly/LaunchDarkly/LDClient.swift +++ b/LaunchDarkly/LaunchDarkly/LDClient.swift @@ -40,11 +40,11 @@ public class LDClient { // MARK: - State Controls and Indicators - private static var instances: [String: LDClient]? - private static let instancesQueue = DispatchQueue(label: "com.launchdarkly.LDClient.instancesQueue") + static var instances: [String: LDClient]? + static let instancesQueue = DispatchQueue(label: "com.launchdarkly.LDClient.instancesQueue") // If the SDK is provided a timeout value that exceeds this value, a warning will be logged. - private static let longTimeoutInterval: TimeInterval = 15 + static let longTimeoutInterval: TimeInterval = 15 /** Reports the online/offline state of the LDClient. @@ -1084,89 +1084,14 @@ public class LDClient { extension LDClient: TypeIdentifying { } -// MARK: - Event delivery -extension LDClient { - private func onEventSyncComplete(result: SynchronizingError?) { +private extension LDClient { + func onEventSyncComplete(result: SynchronizingError?) { if let synchronizingError = result { os_log("%s result: %s", log: config.logger, type: .debug, typeName(and: #function), String(describing: synchronizingError)) } 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 for the result. - - The timeout bounds the wait, not the delivery: an in-flight request is left to finish. Multiple environments - share one budget. - - 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 within 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) - } - - 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 { - let remaining = max(0, deadline.timeIntervalSinceNow) - delivered = client.internalFlushAndWait(timeout: remaining) && delivered - } - return delivered - } - - private func internalFlush() { - eventReporter.flush(completion: nil) - } - - /// 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 { - 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() - } - ) - // A little past the budget, so TimeoutExecutor's timer decides the outcome. - let answered = finished.wait(timeout: .now() + timeout + 0.25) - // `delivered` is only safe to read once the semaphore was signaled. - return answered == .success && delivered - } } #if DEBUG diff --git a/LaunchDarkly/LaunchDarkly/LDClientFlush.swift b/LaunchDarkly/LaunchDarkly/LDClientFlush.swift new file mode 100644 index 000000000..e68286c6e --- /dev/null +++ b/LaunchDarkly/LaunchDarkly/LDClientFlush.swift @@ -0,0 +1,79 @@ +import Foundation +import OSLog + +// MARK: - Flush +extension LDClient { + /** + 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 for the result. + + The timeout bounds the wait, not the delivery: an in-flight request is left to finish. Multiple environments + share one budget. + + 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 within 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) + } + + 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 { + let remaining = max(0, deadline.timeIntervalSinceNow) + delivered = client.internalFlushAndWait(timeout: remaining) && delivered + } + return delivered + } + + func internalFlush() { + eventReporter.flush(completion: nil) + } + + /// 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 { + 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() + } + ) + // A little past the budget, so TimeoutExecutor's timer decides the outcome. + let answered = finished.wait(timeout: .now() + timeout + 0.25) + // `delivered` is only safe to read once the semaphore was signaled. + return answered == .success && delivered + } +} diff --git a/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift b/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift index 1c2645e41..dd34201b2 100644 --- a/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift +++ b/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift @@ -57,10 +57,11 @@ class EventReporter: EventReporting { /// True from sending a request until its response is handled. Its events have already left `eventStore`. private var isDelivering = false - /// A delivery was requested while one was in flight. - private var hasWaitingRequest = false - /// Callers answered by the pass after the in-flight delivery. - private var waitingCompletions: [FlushOutcomeClosure] = [] + /// A flush requested while a delivery was in flight, run once that delivery finishes. Later requests join it + /// rather than queueing another. + private var hasPendingFlush = false + /// Callers answered by the pending flush. + private var pendingFlushCompletions: [FlushOutcomeClosure] = [] private var timerQueue = DispatchQueue(label: "com.launchdarkly.EventReporter.timerQueue") private var eventReportTimer: TimeResponding? @@ -148,10 +149,11 @@ class EventReporter: EventReporting { private func reportEvents(completion: FlushOutcomeClosure?) { guard !isDelivering else { - // Answered after the in-flight delivery, which may hold this caller's events. - hasWaitingRequest = true + // Joins the pending flush rather than answering now, since the in-flight delivery may hold this caller's + // events. + hasPendingFlush = true if let completion { - waitingCompletions.append(completion) + pendingFlushCompletions.append(completion) } return } @@ -198,22 +200,22 @@ class EventReporter: EventReporting { } } - /// 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. + /// Ends the in-flight delivery and runs the pending flush, if one was requested. Failed events are dropped, not + /// retried, so a caller of the pending flush succeeds only if both this delivery and the pending flush did. private func finishDelivery(_ delivered: Bool, _ completion: FlushOutcomeClosure?) { isDelivering = false completion?(delivered) - guard hasWaitingRequest + guard hasPendingFlush else { return } - let waiting = waitingCompletions - hasWaitingRequest = false - waitingCompletions = [] + let completions = pendingFlushCompletions + hasPendingFlush = false + pendingFlushCompletions = [] // 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 || nothingPending)) } + completions.forEach { $0(delivered && (result || nothingPending)) } } } diff --git a/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift index 3f8b74167..0c51f1ada 100644 --- a/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift +++ b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift @@ -146,7 +146,7 @@ final class EventReporterSpec: QuickSpec { expect(delivered).toEventually(beFalse(), timeout: .seconds(10)) } - it("reports true to a waiter when the in-flight delivery is permanently refused") { + it("reports true to a pending flush when the in-flight delivery is permanently refused") { testContext = TestContext() let unauthorized = HTTPURLResponse(url: testContext.serviceMock.config.eventsUrl, statusCode: HTTPURLResponse.StatusCodes.unauthorized, From f14f1de919def53115aa8900827484c856d25a65 Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Thu, 8 Oct 2026 07:39:48 -0700 Subject: [PATCH 06/11] fix(events): Deliver every environment against one shared flush budget Also gates the background flush to platforms that suspend, holds one background assertion per client, and reports flushAndWait's outcome as Android does: true only when LaunchDarkly accepted the events. Co-authored-by: Cursor --- LaunchDarkly.xcodeproj/project.pbxproj | 4 + LaunchDarkly/LaunchDarkly/LDClient.swift | 10 ++- LaunchDarkly/LaunchDarkly/LDClientFlush.swift | 75 ++++++++++------- .../ObjectiveC/ObjcLDClient.swift | 9 ++- .../ServiceObjects/BackgroundActivity.swift | 64 +++++++++++++-- .../ServiceObjects/EventReporter.swift | 48 ++++++----- .../LaunchDarklyTests/LDClientSpec.swift | 27 ++++++- .../BackgroundActivitySpec.swift | 80 +++++++++++++++++++ .../ServiceObjects/EventReporterSpec.swift | 58 +++++++++++++- 9 files changed, 312 insertions(+), 63 deletions(-) create mode 100644 LaunchDarkly/LaunchDarklyTests/ServiceObjects/BackgroundActivitySpec.swift diff --git a/LaunchDarkly.xcodeproj/project.pbxproj b/LaunchDarkly.xcodeproj/project.pbxproj index 16fb0ed2b..d05199fee 100644 --- a/LaunchDarkly.xcodeproj/project.pbxproj +++ b/LaunchDarkly.xcodeproj/project.pbxproj @@ -90,6 +90,7 @@ 831AAE2D20A9E4F600B46DBA /* Throttler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 831AAE2B20A9E4F600B46DBA /* Throttler.swift */; }; 831AAE2E20A9E4F600B46DBA /* Throttler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 831AAE2B20A9E4F600B46DBA /* Throttler.swift */; }; 831AAE3020A9E75D00B46DBA /* ThrottlerSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = 831AAE2F20A9E75D00B46DBA /* ThrottlerSpec.swift */; }; + 3F9AC0D80849C86866E03807 /* BackgroundActivitySpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = C3BDE2A66C982198D55602DA /* BackgroundActivitySpec.swift */; }; 831D2AAF2061AAA000B4AC3C /* Thread.swift in Sources */ = {isa = PBXBuildFile; fileRef = 831D2AAE2061AAA000B4AC3C /* Thread.swift */; }; 831D8B6F1F71532300ED65E8 /* HTTPHeaders.swift in Sources */ = {isa = PBXBuildFile; fileRef = 831D8B6E1F71532300ED65E8 /* HTTPHeaders.swift */; }; 831D8B721F71D3E700ED65E8 /* DarklyService.swift in Sources */ = {isa = PBXBuildFile; fileRef = 831D8B711F71D3E700ED65E8 /* DarklyService.swift */; }; @@ -515,6 +516,7 @@ 831425B0206B030100F2EF36 /* EnvironmentReporter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EnvironmentReporter.swift; sourceTree = ""; }; 831AAE2B20A9E4F600B46DBA /* Throttler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Throttler.swift; sourceTree = ""; }; 831AAE2F20A9E75D00B46DBA /* ThrottlerSpec.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ThrottlerSpec.swift; sourceTree = ""; }; + C3BDE2A66C982198D55602DA /* BackgroundActivitySpec.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BackgroundActivitySpec.swift; sourceTree = ""; }; 831D2AAE2061AAA000B4AC3C /* Thread.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Thread.swift; sourceTree = ""; }; 831D8B6E1F71532300ED65E8 /* HTTPHeaders.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HTTPHeaders.swift; sourceTree = ""; }; 831D8B711F71D3E700ED65E8 /* DarklyService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DarklyService.swift; sourceTree = ""; }; @@ -739,6 +741,7 @@ 83383A5020460DD30024D975 /* SynchronizingErrorSpec.swift */, 837406D321F760640087B22B /* LDTimerSpec.swift */, 831AAE2F20A9E75D00B46DBA /* ThrottlerSpec.swift */, + C3BDE2A66C982198D55602DA /* BackgroundActivitySpec.swift */, 8354AC75224316C700CDE602 /* Cache */, A3C6F7612B7FA803005B3B61 /* SheddingQueueSpec.swift */, ); @@ -1750,6 +1753,7 @@ A3BA7D042BD2BD620000DB28 /* TestContext.swift in Sources */, 83B6E3F1222EFA3800FF2A6A /* ThreadSpec.swift in Sources */, 831AAE3020A9E75D00B46DBA /* ThrottlerSpec.swift in Sources */, + 3F9AC0D80849C86866E03807 /* BackgroundActivitySpec.swift in Sources */, 832D68AC224B3321005F052A /* CacheConverterSpec.swift in Sources */, 838F96741FB9F024009CFC45 /* LDClientSpec.swift in Sources */, 83A0E6B1203B557F00224298 /* FeatureFlagSpec.swift in Sources */, diff --git a/LaunchDarkly/LaunchDarkly/LDClient.swift b/LaunchDarkly/LaunchDarkly/LDClient.swift index 4fbe8c0e0..21c62fd94 100644 --- a/LaunchDarkly/LaunchDarkly/LDClient.swift +++ b/LaunchDarkly/LaunchDarkly/LDClient.swift @@ -263,8 +263,10 @@ public class LDClient { @objc private func didEnterBackground() { os_log("%s", log: config.logger, type: .debug, typeName(and: #function)) - // A backgrounded process is suspended once idle, so deliver inside an activity assertion. - BackgroundActivity.run(reason: "LaunchDarkly event delivery") { [weak self] finished in + // A backgrounded process is suspended once idle, so deliver inside an activity assertion. Only where suspension + // is real: macOS posts this notification whenever the application loses focus, and never suspends it for that. + #if os(iOS) || os(tvOS) + backgroundDelivery.run { [weak self] finished in guard let self = self else { finished() @@ -277,6 +279,7 @@ public class LDClient { finished() } } + #endif Thread.performOnMain { runMode = .background } @@ -289,6 +292,9 @@ public class LDClient { } } + /// One assertion per client, however many times the application is backgrounded while a delivery is running. + private let backgroundDelivery = BackgroundActivity(reason: "LaunchDarkly event delivery") + let config: LDConfig /// Identifies this client's environment to a hook, without handing it the mobile key that identifies the /// environment to LaunchDarkly. Hashed once here rather than per evaluation, because a deduping hook asks for it on diff --git a/LaunchDarkly/LaunchDarkly/LDClientFlush.swift b/LaunchDarkly/LaunchDarkly/LDClientFlush.swift index e68286c6e..6dc47ec65 100644 --- a/LaunchDarkly/LaunchDarkly/LDClientFlush.swift +++ b/LaunchDarkly/LaunchDarkly/LDClientFlush.swift @@ -20,13 +20,18 @@ extension LDClient { Sends any currently queued events to LaunchDarkly and waits up to `timeout` seconds for the result. The timeout bounds the wait, not the delivery: an in-flight request is left to finish. Multiple environments - share one budget. + deliver at the same time and share the one budget, so the call takes no longer for several of them than for one. 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 within the budget. + - returns: `true` if LaunchDarkly accepted the events, or there were none to send. `false` if the timeout expired + first, the client is offline or closed, or any of the events were lost: refused by LaunchDarkly, still failing + after one retry, or unable to be serialized. Lost events are not kept for a later flush, so calling this again + does not resend them. A refusal also stops event delivery until the client is next set online, and until then + this returns `false` even with nothing to send. A `false` because the timeout expired does not mean the events + were not sent: the delivery is left running, and may still arrive. */ @discardableResult public func flushAndWait(timeout: TimeInterval) -> Bool { @@ -39,11 +44,16 @@ extension LDClient { 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)) + let deadline = DispatchTime.now() + max(0, timeout) + // Every environment is started before any of them is waited on. Each has its own event reporter, so waiting on + // one before starting the next would spend the caller's budget on deliveries that could have been running all + // along, and leave the last environment with none of it. + let deliveries = clients.map { $0.startFlush() } var delivered = true - for client in clients { - let remaining = max(0, deadline.timeIntervalSinceNow) - delivered = client.internalFlushAndWait(timeout: remaining) && delivered + for delivery in deliveries { + // Each wait gets what is left of the one budget rather than a fresh copy of it, so that the timeout the + // caller asked for is the time this call can take. + delivered = delivery.outcome(waitingUntil: deadline) && delivered } return delivered } @@ -52,28 +62,37 @@ extension LDClient { eventReporter.flush(completion: nil) } - /// 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 startFlush() -> FlushDelivery { + let delivery = FlushDelivery() + eventReporter.flushReportingOutcome { delivery.finish($0) } + return delivery + } +} - 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() - } - ) - // A little past the budget, so TimeoutExecutor's timer decides the outcome. - let answered = finished.wait(timeout: .now() + timeout + 0.25) - // `delivered` is only safe to read once the semaphore was signaled. - return answered == .success && delivered +/// One environment's delivery, started and not yet waited on. +private final class FlushDelivery { + private let finished = DispatchSemaphore(value: 0) + private let lock = UnfairLock() + /// Only to be used while holding `lock`, because the delivery answers from whichever thread it ended on. + private var delivered = false + + func finish(_ result: Bool) { + lock.lock() + delivered = result + lock.unlock() + finished.signal() + } + + /// How the delivery went, or `false` if it has not answered by `deadline`. + /// + /// A delivery still running when the caller stops waiting is left to finish rather than canceled: its events are + /// already in flight, so abandoning it would only make losing them certain. + func outcome(waitingUntil deadline: DispatchTime) -> Bool { + guard finished.wait(timeout: deadline) == .success + else { return false } + + lock.lock() + defer { lock.unlock() } + return delivered } } diff --git a/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift b/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift index 5205cd575..182e9db85 100644 --- a/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift +++ b/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift @@ -542,12 +542,15 @@ public final class ObjcLDClient: NSObject { /** 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. + - returns: `YES` if LaunchDarkly accepted the events, or there were none to send. `NO` if the timeout expired + first, the client is offline or closed, or any of the events were lost: refused by LaunchDarkly, still failing + after one retry, or unable to be serialized. Lost events are not kept for a later flush, so calling this again + does not resend them. A refusal also stops event delivery until the client is next set online, and until then + this returns `NO` even with nothing to send. A `NO` because the timeout expired does not mean the events were + not sent: the delivery is left running, and may still arrive. */ @objc public func flushAndWait(timeout: TimeInterval) -> Bool { ldClient.flushAndWait(timeout: timeout) diff --git a/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift b/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift index 101273954..e7f5c712a 100644 --- a/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift +++ b/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift @@ -4,13 +4,67 @@ import Foundation /// /// 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 { +/// +/// Holds at most one assertion at a time. Each blocks a system thread until its work finishes, so a burst of +/// backgroundings would otherwise hold one thread each. A request arriving while one is held does not start another: +/// the held assertion runs the work once more after its current pass, which covers whatever was recorded since that +/// pass began. +final class BackgroundActivity { + typealias Work = (_ finished: @escaping () -> Void) -> Void + typealias Assertion = (_ reason: String, _ whileHeld: @escaping Work) -> Void + /// Upper bound for work that never calls `finished`. private static let maximumDuration: TimeInterval = 30 + private let reason: String + private let assertion: Assertion + private let lock = UnfairLock() + /// Only to be used while holding `lock`. + private var isHeld = false + /// The work to run again before the held assertion is released. Only to be used while holding `lock`. + private var rerun: Work? + + init(reason: String, assertion: @escaping Assertion = BackgroundActivity.holdProcessAssertion) { + self.reason = reason + self.assertion = assertion + } + /// 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) { + /// `maximumDuration` passes. + func run(_ work: @escaping Work) { + lock.lock() + guard !isHeld + else { + rerun = work + lock.unlock() + return + } + isHeld = true + lock.unlock() + + assertion(reason) { finished in + self.runHoldingAssertion(work, finished) + } + } + + private func runHoldingAssertion(_ work: @escaping Work, _ finished: @escaping () -> Void) { + work { + self.lock.lock() + guard let next = self.rerun + else { + self.isHeld = false + self.lock.unlock() + finished() + return + } + self.rerun = nil + self.lock.unlock() + self.runHoldingAssertion(next, finished) + } + } + + /// Without assertions on this platform, `whileHeld` runs on the calling thread. + private static func holdProcessAssertion(reason: String, whileHeld: @escaping Work) { #if os(iOS) || os(tvOS) || os(watchOS) let finished = DispatchSemaphore(value: 0) ProcessInfo.processInfo.performExpiringActivity(withReason: reason) { expired in @@ -20,12 +74,12 @@ enum BackgroundActivity { finished.signal() return } - work { finished.signal() } + whileHeld { finished.signal() } // The assertion lasts as long as this block, which runs on a system queue, not main. _ = finished.wait(timeout: .now() + maximumDuration) } #else - work {} + whileHeld {} #endif } } diff --git a/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift b/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift index dd34201b2..27dd411fe 100644 --- a/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift +++ b/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift @@ -2,7 +2,7 @@ import Foundation import OSLog typealias EventSyncCompleteClosure = ((SynchronizingError?) -> Void) -/// Reports whether the events a flush covered left the SDK's hands. +/// Reports whether LaunchDarkly accepted every event a flush covered. typealias FlushOutcomeClosure = (Bool) -> Void // sourcery: autoMockable protocol EventReporting { @@ -16,8 +16,12 @@ protocol EventReporting { func recordFlagEvaluationEvents(flagKey: LDFlagKey, value: LDValue, defaultValue: LDValue, featureFlag: FeatureFlag?, context: LDContext, includeReason: Bool) func flush(completion: CompletionClosure?) - /// Like `flush`. Reports `true` if events were delivered, permanently refused, or absent; `false` if offline or - /// delivery failed. + /// Like `flush`. Reports `true` if LaunchDarkly accepted the events, or there were none to send. Reports `false` if + /// the reporter is offline, or any of the events were lost: refused, still failing after the retry, or unable to + /// be serialized. + /// + /// The completion, like `flush`'s, runs on the reporter's queue, which every recorded event also waits on. Keep it + /// short, and do not record events from it: that would wait on the queue it is already running on. func flushReportingOutcome(completion: @escaping FlushOutcomeClosure) } @@ -212,33 +216,31 @@ class EventReporter: EventReporting { let completions = pendingFlushCompletions hasPendingFlush = false pendingFlushCompletions = [] - // 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 - completions.forEach { $0(delivered && (result || nothingPending)) } + completions.forEach { $0(delivered && result) } } } private func publish(_ events: [Event], _ payloadId: String, _ completion: FlushOutcomeClosure?) { - guard let eventData = encode(events) + guard let (eventData, isComplete) = 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)) - // Encoding is deterministic, so no retry would succeed. - completion?(true) + // Encoding is deterministic, so no retry would succeed; the events are lost. + completion?(false) return } self.service.publishEventData(eventData, payloadId) { response in switch self.processEventResponse(sentEvents: events.count, response: response.urlResponse as? HTTPURLResponse, error: response.error, isRetry: false) { - case .settled: - completion?(true) - case .dropped: + case .accepted: + completion?(isComplete) + case .refused, .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 let outcome = self.processEventResponse(sentEvents: events.count, response: response.urlResponse as? HTTPURLResponse, error: response.error, isRetry: true) - completion?(outcome == .settled) + completion?(outcome == .accepted && isComplete) } } } @@ -246,8 +248,9 @@ class EventReporter: EventReporting { } private enum DeliveryOutcome { - /// Accepted or permanently refused. - case settled + case accepted + /// Refused with a status that no retry would change. The events are dropped. + case refused case retryable /// Failed with no retry left. case dropped @@ -256,12 +259,13 @@ class EventReporter: EventReporting { /// Encodes a run of events as the array the events endpoint takes. /// /// If the run cannot be encoded as a whole, each event is retried on its own. Events that fail are dropped rather - /// than put back, because encoding is deterministic and they would fail every later flush too. - private func encode(_ events: [Event]) -> Data? { + /// than put back, because encoding is deterministic and they would fail every later flush too. `isComplete` is + /// false when any were dropped. + private func encode(_ events: [Event]) -> (data: Data, isComplete: Bool)? { guard encoding != .codable else { if let data = try? encoder.encode(events) { - return data + return (data, true) } return encodeSkippingFailures(events, using: { try encoder.encode($0) }) } @@ -274,7 +278,7 @@ class EventReporter: EventReporting { }) } - private func encodeSkippingFailures(_ events: [Event], using encodeOne: (Event) throws -> Data) -> Data? { + private func encodeSkippingFailures(_ events: [Event], using encodeOne: (Event) throws -> Data) -> (data: Data, isComplete: Bool)? { var payload = Data([UInt8(ascii: "[")]) var written = 0 for event in events { @@ -298,7 +302,7 @@ class EventReporter: EventReporting { guard written > 0 else { return nil } payload.append(UInt8(ascii: "]")) - return payload + return (payload, written == events.count) } private enum EventEncodingError: Error { @@ -314,14 +318,14 @@ 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 .settled + return .accepted } if let statusCode = response?.statusCode, HTTPURLResponse.StatusCodes.isTerminalStatusCode(statusCode) { os_log("%s dropping events due to non-retriable response: %s", log: service.config.logger, type: .debug, typeName(and: #function), String(describing: response)) isOnline = false self.reportSyncComplete(.response(response)) - return .settled + return .refused } 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)) diff --git a/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift b/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift index e226f27cb..82a32fd5c 100644 --- a/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift +++ b/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift @@ -1360,12 +1360,18 @@ 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") { + it("tries to deliver what it has before the process is suspended, where it is suspended") { let testContext = TestContext(startOnline: true, enableBackgroundUpdates: false) testContext.start() NotificationCenter.default.post(name: SystemCapabilities.backgroundNotification!, object: self) + #if os(iOS) || os(tvOS) expect(testContext.eventReporterMock.flushReportingOutcomeCallCount).toEventually(equal(1)) + #else + // macOS posts this on every loss of focus, which suspends nothing. + expect(testContext.subject.runMode).toEventually(equal(LDClientRunMode.background)) + expect(testContext.eventReporterMock.flushReportingOutcomeCallCount) == 0 + #endif } it("background updates enabled") { let testContext = TestContext(startOnline: true) @@ -1679,6 +1685,25 @@ final class LDClientSpec: QuickSpec { expect(testContext.subject.flushAndWait(timeout: 1.0)) == false } + + it("gives every environment the whole budget rather than what the one before it left") { + let testContext = TestContext() + try testContext.config.setSecondaryMobileKeys(["alternate": Constants.alternateMockMobileKey]) + testContext.start() + + // Each takes most of the budget, so delivering them one after another would leave the second + // environment past the deadline before its own delivery had begun. + for environment in ["alternate", LDConfig.Constants.primaryEnvironmentName] { + let reporter = LDClient.get(environment: environment)?.eventReporter as? EventReportingMock + expect(reporter).toNot(beNil()) + reporter?.flushReportingOutcomeCallback = { [weak reporter] in + let completion = reporter?.flushReportingOutcomeReceivedCompletion + DispatchQueue.global().asyncAfter(deadline: .now() + 0.3) { completion?(true) } + } + } + + expect(testContext.subject.flushAndWait(timeout: 0.5)) == true + } } } diff --git a/LaunchDarkly/LaunchDarklyTests/ServiceObjects/BackgroundActivitySpec.swift b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/BackgroundActivitySpec.swift new file mode 100644 index 000000000..a21687ccf --- /dev/null +++ b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/BackgroundActivitySpec.swift @@ -0,0 +1,80 @@ +import Foundation +import Quick +import Nimble +@testable import LaunchDarkly + +final class BackgroundActivitySpec: QuickSpec { + override func spec() { + describe("a background activity") { + var assertions = 0 + var released = 0 + var activity: BackgroundActivity! + beforeEach { + assertions = 0 + released = 0 + activity = BackgroundActivity(reason: "test") { _, work in + assertions += 1 + work { released += 1 } + } + } + + it("holds the assertion until the work finishes") { + var finish: (() -> Void)? + activity.run { finish = $0 } + + expect(assertions) == 1 + expect(released) == 0 + + finish?() + expect(released) == 1 + } + + it("does not start a second assertion while one is held, and runs the later work under the first") { + var finishFirst: (() -> Void)? + var finishSecond: (() -> Void)? + var secondRuns = 0 + activity.run { finishFirst = $0 } + activity.run { finished in + secondRuns += 1 + finishSecond = finished + } + + expect(assertions) == 1 + expect(secondRuns) == 0 + + finishFirst?() + expect(secondRuns) == 1 + expect(released) == 0 + + finishSecond?() + expect(released) == 1 + expect(assertions) == 1 + } + + it("runs the work once more however many requests arrived while it was held") { + var finishFirst: (() -> Void)? + var laterRuns = 0 + activity.run { finishFirst = $0 } + for _ in 0..<3 { + activity.run { finished in + laterRuns += 1 + finished() + } + } + + finishFirst?() + expect(laterRuns) == 1 + expect(assertions) == 1 + expect(released) == 1 + } + + it("starts a new assertion once the held one is released") { + activity.run { $0() } + activity.run { $0() } + + expect(assertions) == 2 + expect(released) == 2 + } + } + } +} diff --git a/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift index 0c51f1ada..44efb5f82 100644 --- a/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift +++ b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift @@ -146,7 +146,61 @@ final class EventReporterSpec: QuickSpec { expect(delivered).toEventually(beFalse(), timeout: .seconds(10)) } - it("reports true to a pending flush when the in-flight delivery is permanently refused") { + it("reports false when LaunchDarkly permanently refuses the events, and goes offline") { + 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 + + var delivered: Bool? + waitUntil { done in + testContext.eventReporter.flushReportingOutcome { result in + delivered = result + done() + } + } + expect(delivered) == false + expect(testContext.eventReporter.isOnline) == false + } + + it("reports false when no event could be serialized") { + testContext = TestContext() + testContext.eventReporter.record(CustomEvent(key: "poison", context: LDContext.stub(), metricValue: .nan)) + testContext.eventReporter.isOnline = true + + var delivered: Bool? + waitUntil { done in + testContext.eventReporter.flushReportingOutcome { result in + delivered = result + done() + } + } + expect(delivered) == false + expect(testContext.serviceMock.publishEventDataCallCount) == 0 + } + + it("reports false when some events could not be serialized, though the rest were accepted") { + testContext = TestContext() + testContext.eventReporter.record(CustomEvent(key: "poison", context: LDContext.stub(), metricValue: .nan)) + testContext.eventReporter.record(CustomEvent(key: "kept", context: LDContext.stub(), metricValue: 1.0)) + testContext.eventReporter.isOnline = true + + var delivered: Bool? + waitUntil { done in + testContext.eventReporter.flushReportingOutcome { result in + delivered = result + done() + } + } + expect(delivered) == false + expect(testContext.serviceMock.publishEventDataCallCount) == 1 + } + + it("reports false to a pending flush when the in-flight delivery is permanently refused") { testContext = TestContext() let unauthorized = HTTPURLResponse(url: testContext.serviceMock.config.eventsUrl, statusCode: HTTPURLResponse.StatusCodes.unauthorized, @@ -167,7 +221,7 @@ final class EventReporterSpec: QuickSpec { testContext.serviceMock.holdsEventCompletions = false testContext.serviceMock.releaseHeldEventCompletions() - expect(delivered).toEventually(beTrue()) + expect(delivered).toEventually(beFalse()) expect(testContext.eventReporter.isOnline) == false } } From 3224c623eb58e67af564e8ddb00147b36d28d43f Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Thu, 8 Oct 2026 14:03:43 -0700 Subject: [PATCH 07/11] fix(events): Keep draining while a delivery is in flight A delivery that had started held back every later one, including the periodic timer's, until it answered. On the retryable path that is about twice the connection timeout, and nothing emptied the event store for the whole of it, so a busy application on a flaky network lost events at the capacity it was supposed to be buffered by. Deliveries now coalesce only while queued and not yet started, which is what Android's DirectEventProcessor does: runDelivery clears the queued waiters before it takes the store, so anything arriving after that gets a delivery of its own. A flush still joins one that has not started, since that delivery will carry its events too. An answer is now always about the batch the caller's own delivery took. That retires the inherited failure a waiter used to pick up from the in-flight batch, which could report events lost that had in fact landed. Completions are also answered off the reporter's queue, uniformly. They had been split between it and whichever thread the delivery ended on, so recording from a completion deadlocked, but only when the event store happened to be empty. Also from review: - Report false from flushAndWait for a client a later start replaced, rather than another client's outcome. - Answer publishEventData where there is no mobile key, rather than dropping the completion. - Keep a flush nobody waits on from adding a waiter. - Hold no assertion around work that does not need one in BackgroundActivity, which drops the refusal case entirely. - Match the Objective-C flushAndWait doc to the Swift one, and say on both that a definitive failure is reported as a failure, since other LaunchDarkly SDKs report it as a success. - Guard the event-publish state in DarklyServiceMock, which concurrent deliveries now reach from more than one thread. Co-authored-by: Cursor --- LaunchDarkly/LaunchDarkly/LDClientFlush.swift | 88 ++++++------ .../Networking/DarklyService.swift | 7 +- .../ObjectiveC/ObjcLDClient.swift | 16 ++- .../ServiceObjects/BackgroundActivity.swift | 8 +- .../ServiceObjects/EventReporter.swift | 135 ++++++++++++------ .../LaunchDarklyTests/LDClientSpec.swift | 34 +++++ .../Mocks/DarklyServiceMock.swift | 68 ++++++--- .../Networking/DarklyServiceSpec.swift | 8 +- .../ServiceObjects/EventReporterSpec.swift | 100 +++++++++---- 9 files changed, 324 insertions(+), 140 deletions(-) diff --git a/LaunchDarkly/LaunchDarkly/LDClientFlush.swift b/LaunchDarkly/LaunchDarkly/LDClientFlush.swift index 6dc47ec65..231e1e0da 100644 --- a/LaunchDarkly/LaunchDarkly/LDClientFlush.swift +++ b/LaunchDarkly/LaunchDarkly/LDClientFlush.swift @@ -16,16 +16,28 @@ extension LDClient { } } + /// Every environment's client, or none where this client is no longer one of them. + private func instancesIncludingThisClient() -> [LDClient] { + let clients = LDClient.instancesQueue.sync { LDClient.instances.map { Array($0.values) } } ?? [] + return clients.contains { $0 === self } ? clients : [] + } + /** Sends any currently queued events to LaunchDarkly and waits up to `timeout` seconds for the result. The timeout bounds the wait, not the delivery: an in-flight request is left to finish. Multiple environments deliver at the same time and share the one budget, so the call takes no longer for several of them than for one. + A delivery that definitively failed is reported as a failure. Some other LaunchDarkly SDKs report that as a + success, on the grounds that the attempt is over, so an expectation carried from another platform may not hold + here. + 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. + - parameter timeout: How long to wait, in seconds. Zero or less does not wait, and so reports `false`. There is + no upper limit: a delivery always ends, by its own request timeouts if nothing else, so a very long timeout + waits for that. - returns: `true` if LaunchDarkly accepted the events, or there were none to send. `false` if the timeout expired first, the client is offline or closed, or any of the events were lost: refused by LaunchDarkly, still failing after one retry, or unable to be serialized. Lost events are not kept for a later flush, so calling this again @@ -39,60 +51,52 @@ 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) } - guard let clients = LDClient.instancesQueue.sync(execute: { LDClient.instances.map { Array($0.values) } }) + let clients = instancesIncludingThisClient() + guard !clients.isEmpty else { + // Closed, or replaced by a later start, or caught while a start is still filling the instances in. None of + // them can deliver this client's events, and saying otherwise would tell the caller they were safe. os_log("%s called on a closed client", log: config.logger, type: .debug, self.typeName(and: #function)) return false } let deadline = DispatchTime.now() + max(0, timeout) + // Counts answers rather than tracking each delivery, so that a reporter answering twice costs a spare permit + // no one reads. A DispatchGroup would fit as well, but its `leave` traps without a matching `enter`, which + // turns a broken reporter into a crash in the application. + let answered = DispatchSemaphore(value: 0) + let lock = UnfairLock() + /// Only to be used while holding `lock`: each environment answers from whichever thread its delivery ended on. + var delivered = true + // Every environment is started before any of them is waited on. Each has its own event reporter, so waiting on // one before starting the next would spend the caller's budget on deliveries that could have been running all // along, and leave the last environment with none of it. - let deliveries = clients.map { $0.startFlush() } - var delivered = true - for delivery in deliveries { - // Each wait gets what is left of the one budget rather than a fresh copy of it, so that the timeout the - // caller asked for is the time this call can take. - delivered = delivery.outcome(waitingUntil: deadline) && delivered + for client in clients { + client.eventReporter.flushReportingOutcome { result in + lock.lock() + delivered = delivered && result + lock.unlock() + answered.signal() + } } - return delivered - } - - func internalFlush() { - eventReporter.flush(completion: nil) - } - private func startFlush() -> FlushDelivery { - let delivery = FlushDelivery() - eventReporter.flushReportingOutcome { delivery.finish($0) } - return delivery - } -} - -/// One environment's delivery, started and not yet waited on. -private final class FlushDelivery { - private let finished = DispatchSemaphore(value: 0) - private let lock = UnfairLock() - /// Only to be used while holding `lock`, because the delivery answers from whichever thread it ended on. - private var delivered = false - - func finish(_ result: Bool) { - lock.lock() - delivered = result - lock.unlock() - finished.signal() - } - - /// How the delivery went, or `false` if it has not answered by `deadline`. - /// - /// A delivery still running when the caller stops waiting is left to finish rather than canceled: its events are - /// already in flight, so abandoning it would only make losing them certain. - func outcome(waitingUntil deadline: DispatchTime) -> Bool { - guard finished.wait(timeout: deadline) == .success - else { return false } + // Every wait is against the one deadline rather than a fresh copy of it, so the timeout the caller asked for + // is the time this call can take however many environments there are. A delivery still running when the wait + // gives up is left to finish rather than canceled: its events are already in flight, so abandoning it would + // only make losing them certain. + for _ in clients { + guard answered.wait(timeout: deadline) == .success + else { return false } + } + // The waits count answers rather than environments, so a reporter answering twice can end them while another + // environment's answer is still being written. lock.lock() defer { lock.unlock() } return delivered } + + func internalFlush() { + eventReporter.flush(completion: nil) + } } diff --git a/LaunchDarkly/LaunchDarkly/Networking/DarklyService.swift b/LaunchDarkly/LaunchDarkly/Networking/DarklyService.swift index 7d980c859..e49b7ca27 100644 --- a/LaunchDarkly/LaunchDarkly/Networking/DarklyService.swift +++ b/LaunchDarkly/LaunchDarkly/Networking/DarklyService.swift @@ -189,7 +189,12 @@ final class DarklyService: DarklyServiceProvider { // MARK: Publish Events func publishEventData(_ eventData: Data, _ payloadId: String, completion: ServiceCompletionHandler?) { - guard hasMobileKey(#function) else { return } + guard hasMobileKey(#function) + else { + // Answered rather than dropped: the reporter holds every later delivery until this one answers. + completion?((nil, nil, URLError(.userAuthenticationRequired), nil)) + return + } let url = config.eventsUrl.appendingPathComponent(EventRequestPath.bulk) let headers = [HTTPHeaders.HeaderKey.eventPayloadIDHeader: payloadId].merging(httpHeaders.eventRequestHeaders) { $1 } doPublish(url: url, headers: headers, body: eventData, completion: completion) diff --git a/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift b/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift index 182e9db85..9e19e09a7 100644 --- a/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift +++ b/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift @@ -540,11 +540,21 @@ public final class ObjcLDClient: NSObject { } /** - Sends any currently queued events and waits up to `timeout` for the delivery to finish. + Sends any currently queued events to LaunchDarkly and waits up to `timeout` seconds for the result. - This is not a crash-time mechanism. Keep the budget far below 15 seconds when calling from the main thread. + The timeout bounds the wait, not the delivery: an in-flight request is left to finish. Multiple environments + deliver at the same time and share the one budget, so the call takes no longer for several of them than for one. - - parameter timeout: How long to wait, in seconds. + A delivery that definitively failed is reported as a failure. Some other LaunchDarkly SDKs report that as a + success, on the grounds that the attempt is over, so an expectation carried from another platform may not hold + here. + + 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. Zero or less does not wait, and so reports `NO`. There is + no upper limit: a delivery always ends, by its own request timeouts if nothing else, so a very long timeout + waits for that. - returns: `YES` if LaunchDarkly accepted the events, or there were none to send. `NO` if the timeout expired first, the client is offline or closed, or any of the events were lost: refused by LaunchDarkly, still failing after one retry, or unable to be serialized. Lost events are not kept for a later flush, so calling this again diff --git a/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift b/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift index e7f5c712a..c31fed7c9 100644 --- a/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift +++ b/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift @@ -67,17 +67,19 @@ final class BackgroundActivity { private static func holdProcessAssertion(reason: String, whileHeld: @escaping Work) { #if os(iOS) || os(tvOS) || os(watchOS) let finished = DispatchSemaphore(value: 0) + // The work does not wait on the assertion, and does not run inside it. The system refuses one outright under + // the very resource pressure this is here for, and a delivery it may suspend is better than no delivery. ProcessInfo.processInfo.performExpiringActivity(withReason: reason) { expired in guard !expired else { - // Expiration: release the blocked first call. + // The system taking the assertion back, or declining to give one. Nothing should stay blocked on it. finished.signal() return } - whileHeld { finished.signal() } - // The assertion lasts as long as this block, which runs on a system queue, not main. + // The assertion lasts as long as this block, which runs on a system queue, not the caller's thread. _ = finished.wait(timeout: .now() + maximumDuration) } + whileHeld { finished.signal() } #else whileHeld {} #endif diff --git a/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift b/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift index 27dd411fe..451c7edc5 100644 --- a/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift +++ b/LaunchDarkly/LaunchDarkly/ServiceObjects/EventReporter.swift @@ -20,8 +20,13 @@ protocol EventReporting { /// the reporter is offline, or any of the events were lost: refused, still failing after the retry, or unable to /// be serialized. /// - /// The completion, like `flush`'s, runs on the reporter's queue, which every recorded event also waits on. Keep it - /// short, and do not record events from it: that would wait on the queue it is already running on. + /// Flushes arriving faster than a delivery completes join the one that is queued, since a delivery that has not + /// started yet will send their events too. A delivery that has already started is not joined, so an answer is + /// always about the batch the caller's own delivery took, never one an earlier delivery lost. + /// + /// The completion, like `flush`'s, runs on a background queue, and never on the reporter's own: a completion is + /// free to record events or flush again without waiting on the queue it is running on. It is not the caller's + /// thread either, and two completions may run at once, so anything it touches needs to expect that. func flushReportingOutcome(completion: @escaping FlushOutcomeClosure) } @@ -59,13 +64,16 @@ class EventReporter: EventReporting { private(set) var eventStore: [Event] = [] private(set) var contextSummarizer: ContextSummarizer - /// True from sending a request until its response is handled. Its events have already left `eventStore`. - private var isDelivering = false - /// A flush requested while a delivery was in flight, run once that delivery finishes. Later requests join it - /// rather than queueing another. - private var hasPendingFlush = false - /// Callers answered by the pending flush. - private var pendingFlushCompletions: [FlushOutcomeClosure] = [] + /// Guards `queuedDelivery`. Taken on a caller's thread and on `eventQueue`, and never held across either. + private let deliveryLock = UnfairLock() + /// Callers waiting on a delivery that is queued and has not taken the event store yet, or nil where none is + /// queued. + private var queuedDelivery: [FlushOutcomeClosure]? + + #if DEBUG + /// How many deliveries have started, which is fewer than the flushes asked for where they were joined. + private var deliveryRunCount = 0 + #endif private var timerQueue = DispatchQueue(label: "com.launchdarkly.EventReporter.timerQueue") private var eventReportTimer: TimeResponding? @@ -128,7 +136,7 @@ class EventReporter: EventReporting { private func startReporting() { guard eventReportTimer == nil else { return } - eventReportTimer = LDTimer(withTimeInterval: service.config.eventFlushInterval, fireQueue: eventQueue, execute: reportEvents) + eventReportTimer = LDTimer(withTimeInterval: service.config.eventFlushInterval, fireQueue: eventQueue, execute: queueScheduledDelivery) } private func stopReporting() { @@ -137,31 +145,69 @@ class EventReporter: EventReporting { } func flush(completion: CompletionClosure?) { - flushReportingOutcome { _ in completion?() } + // Nil stays nil, so a flush nobody waits on joins a queued delivery without adding to its waiters. + queueDelivery(completion.map { done in { _ in done() } }) } func flushReportingOutcome(completion: @escaping FlushOutcomeClosure) { - eventQueue.async { - self.reportEvents(completion: completion) - } + queueDelivery(completion) } - private func reportEvents() { - reportEvents(completion: nil) + private func queueScheduledDelivery() { + queueDelivery(nil) } - private func reportEvents(completion: FlushOutcomeClosure?) { - guard !isDelivering - else { - // Joins the pending flush rather than answering now, since the in-flight delivery may hold this caller's - // events. - hasPendingFlush = true + /// Queues a delivery, or joins one that is queued and has not started. + /// + /// A delivery that has not started yet will send everything recorded before it starts, which includes this + /// caller's events, so joining it is as good as queueing another. Without this, flushes arriving faster than a + /// post completes each queue their own, and the one that matters -- the flush at shutdown -- waits behind all of + /// them. + /// + /// A delivery that has already started is not joined. It has taken the event store, so it cannot speak for + /// anything recorded since, and holding later deliveries back until it answers would leave the store growing + /// through a retry while nothing drained it. + private func queueDelivery(_ completion: FlushOutcomeClosure?) { + deliveryLock.lock() + if queuedDelivery != nil { if let completion { - pendingFlushCompletions.append(completion) + queuedDelivery?.append(completion) } + deliveryLock.unlock() return } + queuedDelivery = completion.map { [$0] } ?? [] + deliveryLock.unlock() + eventQueue.async { self.runDelivery() } + } + + private func runDelivery() { + // Requests arriving from here on need a delivery of their own: this one is about to take the event store, and + // what it takes is all it can speak for. + deliveryLock.lock() + let waiters = queuedDelivery ?? [] + queuedDelivery = nil + #if DEBUG + deliveryRunCount += 1 + #endif + deliveryLock.unlock() + + reportEvents { delivered in + guard !waiters.isEmpty + else { return } + // Answered away from `eventQueue`, and from whichever thread the delivery ended on, so that every + // completion reaches its caller the same way. A completion is free to record or flush again: it is never + // running on the queue those wait for. + DispatchQueue.global().async { + for waiter in waiters { + waiter(delivered) + } + } + } + } + + private func reportEvents(completion: FlushOutcomeClosure?) { guard isOnline else { os_log("%s aborted. EventReporter is offline", log: service.config.logger, type: .debug, typeName(and: #function)) @@ -194,30 +240,8 @@ class EventReporter: EventReporting { service.diagnosticCache?.recordEventsInLastBatch(eventsInLastBatch: toPublish.count) - isDelivering = true DispatchQueue.global().async { - self.publish(toPublish, UUID().uuidString) { delivered in - self.eventQueue.async { - self.finishDelivery(delivered, completion) - } - } - } - } - - /// Ends the in-flight delivery and runs the pending flush, if one was requested. Failed events are dropped, not - /// retried, so a caller of the pending flush succeeds only if both this delivery and the pending flush did. - private func finishDelivery(_ delivered: Bool, _ completion: FlushOutcomeClosure?) { - isDelivering = false - completion?(delivered) - - guard hasPendingFlush - else { return } - - let completions = pendingFlushCompletions - hasPendingFlush = false - pendingFlushCompletions = [] - reportEvents { result in - completions.forEach { $0(delivered && result) } + self.publish(toPublish, UUID().uuidString, completion) } } @@ -384,5 +408,24 @@ extension EventReporter: TypeIdentifying { } func setLastEventResponseDate(_ date: Date) { lastEventResponseDate = date } + + /// Occupies the queue a delivery runs on, so a test can see what one that has not started yet collects. + func occupyQueue(until released: DispatchSemaphore) { + eventQueue.async { _ = released.wait(timeout: .now() + 10) } + } + + /// Callers waiting on the queued delivery. + var queuedDeliveryWaiterCount: Int { + deliveryLock.lock() + defer { deliveryLock.unlock() } + return queuedDelivery?.count ?? 0 + } + + /// Deliveries started, counting one for a run of flushes that joined each other. + var startedDeliveryCount: Int { + deliveryLock.lock() + defer { deliveryLock.unlock() } + return deliveryRunCount + } } #endif diff --git a/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift b/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift index 82a32fd5c..b14bc2086 100644 --- a/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift +++ b/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift @@ -1686,6 +1686,19 @@ final class LDClientSpec: QuickSpec { expect(testContext.subject.flushAndWait(timeout: 1.0)) == false } + it("reports false for a client a later start replaced, without flushing the clients that replaced it") { + let stale = TestContext() + stale.start() + stale.subject.close() + let current = TestContext() + current.start() + answer(current.eventReporterMock, with: true) + + // Its own events were never attempted, so the outcome of another client's delivery says nothing of them. + expect(stale.subject.flushAndWait(timeout: 1.0)) == false + expect(current.eventReporterMock.flushReportingOutcomeCallCount) == 0 + } + it("gives every environment the whole budget rather than what the one before it left") { let testContext = TestContext() try testContext.config.setSecondaryMobileKeys(["alternate": Constants.alternateMockMobileKey]) @@ -1704,6 +1717,27 @@ final class LDClientSpec: QuickSpec { expect(testContext.subject.flushAndWait(timeout: 0.5)) == true } + + it("takes no longer for several environments than the budget it was given") { + let testContext = TestContext() + try testContext.config.setSecondaryMobileKeys(["alternate": Constants.alternateMockMobileKey]) + testContext.start() + // One answers most of the way through the budget and the other never does, so the budget the first + // spent is the budget the second has left. Giving each its own copy of it would take twice as long. + let answering = LDClient.get(environment: "alternate")?.eventReporter as? EventReportingMock + expect(answering).toNot(beNil()) + answering?.flushReportingOutcomeCallback = { [weak answering] in + let completion = answering?.flushReportingOutcomeReceivedCompletion + DispatchQueue.global().asyncAfter(deadline: .now() + 0.5) { completion?(true) } + } + testContext.eventReporterMock.flushReportingOutcomeCallback = nil + + // Answering at 0.5s into a 0.6s budget leaves the second wait 0.1s, against the 0.6s a fresh copy + // would give it, so the two outcomes are 0.6s and 1.1s with room on either side of the threshold. + let started = Date() + expect(testContext.subject.flushAndWait(timeout: 0.6)) == false + expect(Date().timeIntervalSince(started)) < 0.85 + } } } diff --git a/LaunchDarkly/LaunchDarklyTests/Mocks/DarklyServiceMock.swift b/LaunchDarkly/LaunchDarklyTests/Mocks/DarklyServiceMock.swift index 12545b74a..94dca6865 100644 --- a/LaunchDarkly/LaunchDarklyTests/Mocks/DarklyServiceMock.swift +++ b/LaunchDarkly/LaunchDarklyTests/Mocks/DarklyServiceMock.swift @@ -133,30 +133,66 @@ final class DarklyServiceMock: DarklyServiceProvider { return mock } - var stubbedEventResponse: ServiceResponse? - var publishEventDataCallCount = 0 - var publishedEventData: Data? - /// Holds responses so a test can act while a delivery is in flight. - var holdsEventCompletions = false + /// Guards the event-publish state below. Publishes arrive on the reporter's threads, and the retry's, while the + /// test thread sets the stubs and reads the results. + private let eventLock = NSLock() + private var _stubbedEventResponse: ServiceResponse? + private var _publishEventDataCallCount = 0 + private var _publishedEventData: Data? + private var _holdsEventCompletions = false private var heldEventCompletions: [ServiceCompletionHandler] = [] + + var stubbedEventResponse: ServiceResponse? { + get { withEventLock { _stubbedEventResponse } } + set { withEventLock { _stubbedEventResponse = newValue } } + } + var publishEventDataCallCount: Int { + get { withEventLock { _publishEventDataCallCount } } + set { withEventLock { _publishEventDataCallCount = newValue } } + } + var publishedEventData: Data? { + get { withEventLock { _publishedEventData } } + set { withEventLock { _publishedEventData = newValue } } + } + /// Holds responses so a test can act while a delivery is in flight. + var holdsEventCompletions: Bool { + get { withEventLock { _holdsEventCompletions } } + set { withEventLock { _holdsEventCompletions = newValue } } + } + func publishEventData(_ eventData: Data, _ payloadId: String, completion: ServiceCompletionHandler?) { - publishEventDataCallCount += 1 - publishedEventData = eventData - guard !holdsEventCompletions - else { - if let completion { - heldEventCompletions.append(completion) + // Decided under the lock, answered outside it: the completion re-enters the reporter, which may publish again. + let response: ServiceResponse? = withEventLock { + _publishEventDataCallCount += 1 + _publishedEventData = eventData + guard !_holdsEventCompletions + else { + if let completion { + heldEventCompletions.append(completion) + } + return nil } - return + return _stubbedEventResponse ?? (nil, nil, nil, nil) + } + if let response { + completion?(response) } - completion?(stubbedEventResponse ?? (nil, nil, nil, nil)) } /// Answers every held request. func releaseHeldEventCompletions() { - let held = heldEventCompletions - heldEventCompletions = [] - held.forEach { $0(stubbedEventResponse ?? (nil, nil, nil, nil)) } + let (held, response) = withEventLock { () -> ([ServiceCompletionHandler], ServiceResponse) in + let held = heldEventCompletions + heldEventCompletions = [] + return (held, _stubbedEventResponse ?? (nil, nil, nil, nil)) + } + held.forEach { $0(response) } + } + + private func withEventLock(_ body: () -> T) -> T { + eventLock.lock() + defer { eventLock.unlock() } + return body() } var stubbedDiagnosticResponse: ServiceResponse? diff --git a/LaunchDarkly/LaunchDarklyTests/Networking/DarklyServiceSpec.swift b/LaunchDarkly/LaunchDarklyTests/Networking/DarklyServiceSpec.swift index 19a917e48..d7d6d227b 100644 --- a/LaunchDarkly/LaunchDarklyTests/Networking/DarklyServiceSpec.swift +++ b/LaunchDarkly/LaunchDarklyTests/Networking/DarklyServiceSpec.swift @@ -629,10 +629,12 @@ final class DarklyServiceSpec: QuickSpec { eventsPublished = true } } - it("does not make a request") { + it("does not make a request, and still answers with a failure") { expect(eventRequest).to(beNil()) - expect(eventsPublished) == false - expect(responses).to(beNil()) + // A reporter holds every later delivery until this one answers, so it must not be left waiting. + expect(eventsPublished) == true + expect(responses.urlResponse).to(beNil()) + expect(responses.error).toNot(beNil()) } } } diff --git a/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift index 44efb5f82..9aa7d64bd 100644 --- a/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift +++ b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/EventReporterSpec.swift @@ -60,6 +60,27 @@ final class EventReporterSpec: QuickSpec { eventReporter.record(event) } } + + /// Flushes, and answers whether recording from the completion got anywhere. + /// + /// Recording waits on the reporter's queue, so a completion run on that queue cannot record. The recording + /// happens on another thread against a deadline, so that a reporter answering on its own queue fails this + /// rather than hanging the test process on a deadlock it can never leave. + func recordedFromACompletion() -> Bool { + var recorded = false + waitUntil(timeout: .seconds(5)) { done in + eventReporter.flushReportingOutcome { _ in + let finished = DispatchSemaphore(value: 0) + DispatchQueue.global().async { + eventReporter.record(CustomEvent(key: "recorded from a completion", context: LDContext.stub())) + finished.signal() + } + recorded = finished.wait(timeout: .now() + 1) == .success + done() + } + } + return recorded + } } override func spec() { @@ -124,26 +145,23 @@ final class EventReporterSpec: QuickSpec { 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) + it("keeps draining the event store while a delivery is in flight") { + testContext = TestContext() testContext.recordEvents(1) testContext.eventReporter.isOnline = true - // Keep the first delivery in flight with the event store already empty. 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) - expect(delivered).to(beNil()) + // Holding this back until the first delivery answers would let the store fill through a retry, which + // can run for twice the connection timeout, and events recorded past the capacity are lost. + testContext.recordEvents(1) + testContext.eventReporter.flushReportingOutcome { _ in } + expect(testContext.serviceMock.publishEventDataCallCount).toEventually(equal(2)) testContext.serviceMock.holdsEventCompletions = false testContext.serviceMock.releaseHeldEventCompletions() - - // Allows for the one-second retry; the failed events are dropped. - expect(delivered).toEventually(beFalse(), timeout: .seconds(10)) } it("reports false when LaunchDarkly permanently refuses the events, and goes offline") { @@ -200,29 +218,59 @@ final class EventReporterSpec: QuickSpec { expect(testContext.serviceMock.publishEventDataCallCount) == 1 } - it("reports false to a pending flush when the in-flight delivery is permanently refused") { + it("answers away from the queue a recorded event waits on, after a delivery") { 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)) + expect(testContext.recordedFromACompletion()) == true + } - var delivered: Bool? - testContext.eventReporter.flushReportingOutcome { result in delivered = result } - Thread.sleep(forTimeInterval: 0.2) + it("answers away from the queue a recorded event waits on, with nothing to send") { + testContext = TestContext() + testContext.eventReporter.isOnline = true - testContext.serviceMock.holdsEventCompletions = false - testContext.serviceMock.releaseHeldEventCompletions() + // The path that answers without ever leaving the reporter's queue, so it is the one that would + // deadlock while the delivery path, answering from the response, did not. + expect(testContext.recordedFromACompletion()) == true + } - expect(delivered).toEventually(beFalse()) - expect(testContext.eventReporter.isOnline) == false + it("sends one delivery for the flushes that arrive before it starts, and answers them all") { + testContext = TestContext(stubResponseSuccess: false) + testContext.recordEvents(1) + testContext.eventReporter.isOnline = true + let released = DispatchSemaphore(value: 0) + testContext.eventReporter.occupyQueue(until: released) + + // An application flushing in a loop would otherwise queue a delivery each time, leaving the flush + // that matters waiting behind all of them, and growing the waiter list without bound. + for _ in 0..<100 { + testContext.eventReporter.flush(completion: nil) + } + expect(testContext.eventReporter.queuedDeliveryWaiterCount) == 0 + + var delivered: [Bool] = [] + let answers = UnfairLock() + // Both completions run at once, and the poll below reads from a third thread. + let answered: () -> [Bool] = { + answers.lock() + defer { answers.unlock() } + return delivered + } + for _ in 0..<2 { + testContext.eventReporter.flushReportingOutcome { result in + answers.lock() + delivered.append(result) + answers.unlock() + } + } + expect(testContext.eventReporter.queuedDeliveryWaiterCount) == 2 + + released.signal() + // Allows for the one-second retry; the failed events are dropped. + expect(answered()).toEventually(equal([false, false]), timeout: .seconds(10)) + expect(testContext.eventReporter.startedDeliveryCount) == 1 + expect(testContext.serviceMock.publishEventDataCallCount) == 2 // the first attempt and its retry } } } From 07f80c63689075d662b63cc2bf2a8e0675be1bac Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Thu, 8 Oct 2026 14:14:37 -0700 Subject: [PATCH 08/11] feat(events): Report a flush outcome without blocking the caller flushAndWait is the only way to learn whether events reached LaunchDarkly, and it blocks. From a Swift concurrency task that is not safe: the cooperative pool does not grow to replace a thread held that way, so enough of these stall every other task in the process, including whichever one would have finished the delivery. flush(completion:) reports the same outcome over every environment and returns at once. It takes no timeout, because a delivery always ends by its own request timeouts if nothing else, and the caller is not waiting on it in any case. The documentation on all three now says which to reach for, and flushAndWait says outright that it blocks the calling thread and must not be called from a task. Co-authored-by: Cursor --- LaunchDarkly/LaunchDarkly/LDClientFlush.swift | 54 ++++++++++++++++ .../ObjectiveC/ObjcLDClient.swift | 20 ++++++ .../LaunchDarklyTests/LDClientSpec.swift | 63 +++++++++++++++++++ 3 files changed, 137 insertions(+) diff --git a/LaunchDarkly/LaunchDarkly/LDClientFlush.swift b/LaunchDarkly/LaunchDarkly/LDClientFlush.swift index 231e1e0da..d1bff28db 100644 --- a/LaunchDarkly/LaunchDarkly/LDClientFlush.swift +++ b/LaunchDarkly/LaunchDarkly/LDClientFlush.swift @@ -9,6 +9,9 @@ extension LDClient { 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. + + Because it does not wait, it cannot report how the delivery went. Use `flush(completion:)` where that matters + but blocking does not suit, or `flushAndWait` where blocking is acceptable. */ public func flush() { LDClient.instancesQueue.sync(flags: .barrier) { @@ -16,6 +19,49 @@ extension LDClient { } } + /** + Sends any currently queued events to LaunchDarkly and reports how the delivery went. + + Like `flushAndWait`, but without blocking: the completion runs once every environment has answered. There is no + timeout, because a delivery always ends, by its own request timeouts if nothing else. This is the one to reach + for from a Swift concurrency task, where blocking is not safe. + + - parameter completion: Given `true` if LaunchDarkly accepted the events, or there were none to send, and + `false` if the client is offline or closed, or any of the events were lost. See `flushAndWait` for what being + lost covers. Runs once, on a background queue that is neither the caller's thread nor the one recorded events + wait on, so it is free to record or flush again. + */ + public func flush(completion: @escaping (Bool) -> Void) { + let clients = instancesIncludingThisClient() + guard !clients.isEmpty + else { + os_log("%s called on a closed client", log: config.logger, type: .debug, self.typeName(and: #function)) + completion(false) + return + } + + let lock = UnfairLock() + /// Only to be used while holding `lock`: each environment answers from whichever thread its delivery ended on. + var delivered = true + var outstanding = clients.count + + for client in clients { + client.eventReporter.flushReportingOutcome { result in + lock.lock() + delivered = delivered && result + outstanding -= 1 + // Nil until the last environment answers, so the completion runs once. A reporter answering twice + // takes the count past zero rather than back to it, which cannot match again. + let outcome = outstanding == 0 ? delivered : nil + lock.unlock() + + if let outcome { + completion(outcome) + } + } + } + } + /// Every environment's client, or none where this client is no longer one of them. private func instancesIncludingThisClient() -> [LDClient] { let clients = LDClient.instancesQueue.sync { LDClient.instances.map { Array($0.values) } } ?? [] @@ -32,6 +78,14 @@ extension LDClient { success, on the grounds that the attempt is over, so an expectation carried from another platform may not hold here. + This blocks the thread it is called on until the answer is in or the budget runs out. Where the answer is not + needed, `flush` queues the same delivery and returns at once; where it is needed but blocking does not suit, + `flush(completion:)` reports it without waiting. + + Do not call this from a Swift concurrency task. Blocking there holds a thread of the cooperative pool, which + cannot grow to replace it, and enough of them will stall every other task in the process. Use + `flush(completion:)` from a task, wrapped in a continuation where you want to await it. + 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. diff --git a/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift b/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift index 9e19e09a7..46d386954 100644 --- a/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift +++ b/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift @@ -534,11 +534,28 @@ public final class ObjcLDClient: NSObject { 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. + + Because it does not wait, it cannot report how the delivery went. Use `flushWithCompletion:` where that matters + but blocking does not suit, or `flushAndWait` where blocking is acceptable. */ @objc public func flush() { ldClient.flush() } + /** + Sends any currently queued events to LaunchDarkly and reports how the delivery went. + + Like `flushAndWait`, but without blocking: the completion runs once every environment has answered. There is no + timeout, because a delivery always ends, by its own request timeouts if nothing else. + + - parameter completion: Given `YES` if LaunchDarkly accepted the events, or there were none to send, and `NO` if + the client is offline or closed, or any of the events were lost. See `flushAndWait` for what being lost + covers. Runs once, on a background queue that is not the caller's thread. + */ + @objc public func flush(completion: @escaping (Bool) -> Void) { + ldClient.flush(completion: completion) + } + /** Sends any currently queued events to LaunchDarkly and waits up to `timeout` seconds for the result. @@ -549,6 +566,9 @@ public final class ObjcLDClient: NSObject { success, on the grounds that the attempt is over, so an expectation carried from another platform may not hold here. + This blocks the thread it is called on until the answer is in or the budget runs out, so where the answer is + not needed, `flush` queues the same delivery and returns at once. + 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. diff --git a/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift b/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift index b14bc2086..6cac901d5 100644 --- a/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift +++ b/LaunchDarkly/LaunchDarklyTests/LDClientSpec.swift @@ -1644,6 +1644,69 @@ final class LDClientSpec: QuickSpec { } } + describe("flush with a completion") { + /// Answers from another thread, because this call does not block and must not be answered inside it. + func answer(_ mock: EventReportingMock, with delivered: Bool, after delay: TimeInterval = 0.05) { + mock.flushReportingOutcomeCallback = { [weak mock] in + let completion = mock?.flushReportingOutcomeReceivedCompletion + DispatchQueue.global().asyncAfter(deadline: .now() + delay) { completion?(delivered) } + } + } + + it("reports true when delivery succeeds") { + let testContext = TestContext() + testContext.start() + answer(testContext.eventReporterMock, with: true) + + var delivered: Bool? + testContext.subject.flush { delivered = $0 } + // Nothing has answered yet, so a call that reported now would be blocking or guessing. + expect(delivered).to(beNil()) + expect(delivered).toEventually(beTrue()) + } + + it("reports false when delivery cannot finish") { + let testContext = TestContext() + testContext.start() + answer(testContext.eventReporterMock, with: false) + + var delivered: Bool? + testContext.subject.flush { delivered = $0 } + expect(delivered).toEventually(beFalse()) + } + + it("reports false for a client a later start replaced, without flushing the clients that replaced it") { + let stale = TestContext() + stale.start() + stale.subject.close() + let current = TestContext() + current.start() + answer(current.eventReporterMock, with: true) + + var delivered: Bool? + stale.subject.flush { delivered = $0 } + expect(delivered) == false + expect(current.eventReporterMock.flushReportingOutcomeCallCount) == 0 + } + + it("reports once every environment has answered, and only once") { + let testContext = TestContext() + try testContext.config.setSecondaryMobileKeys(["alternate": Constants.alternateMockMobileKey]) + testContext.start() + let answering = LDClient.get(environment: "alternate")?.eventReporter as? EventReportingMock + expect(answering).toNot(beNil()) + // The one that fails answers last, so reporting before every environment is in would say true. + answer(testContext.eventReporterMock, with: true, after: 0.05) + answer(answering!, with: false, after: 0.2) + + var answers: [Bool] = [] + testContext.subject.flush { answers.append($0) } + expect(answers).toEventually(equal([false])) + Thread.sleep(forTimeInterval: 0.2) + expect(answers) == [false] + } + } + describe("flushAndWait") { /// Answers inside the call, because `flushAndWait` blocks the test thread. func answer(_ mock: EventReportingMock, with delivered: Bool) { From 63af7065b583fcefb7a9cc888fb143a09720db57 Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Thu, 8 Oct 2026 14:32:00 -0700 Subject: [PATCH 09/11] test(events): Drive the background assertion from a test, and log a refusal The assertion the process grants was taken directly from ProcessInfo, so nothing could exercise the paths that matter: a system that refuses one outright, and the cap on work that never finishes. The backgrounding test in LDClientSpec was also at the mercy of whether the simulator felt like granting one. processAssertion takes the system call and the cap as arguments, so a test can refuse an assertion, grant one, or let it run past the cap, without the system's cooperation. Three tests cover those, and the refusal one fails if the work is moved back inside the activity block, which is where it would never run under the resource pressure this is here for. The refusal is now logged as well. The delivery goes ahead either way, so it is no longer silent, but a log could not tell a held assertion from one that was never granted. Co-authored-by: Cursor --- LaunchDarkly/LaunchDarkly/LDClient.swift | 3 +- .../ServiceObjects/BackgroundActivity.swift | 57 ++++++++++++----- .../BackgroundActivitySpec.swift | 64 +++++++++++++++++++ 3 files changed, 106 insertions(+), 18 deletions(-) diff --git a/LaunchDarkly/LaunchDarkly/LDClient.swift b/LaunchDarkly/LaunchDarkly/LDClient.swift index 21c62fd94..9aa5b1635 100644 --- a/LaunchDarkly/LaunchDarkly/LDClient.swift +++ b/LaunchDarkly/LaunchDarkly/LDClient.swift @@ -293,7 +293,7 @@ public class LDClient { } /// One assertion per client, however many times the application is backgrounded while a delivery is running. - private let backgroundDelivery = BackgroundActivity(reason: "LaunchDarkly event delivery") + private let backgroundDelivery: BackgroundActivity let config: LDConfig /// Identifies this client's environment to a hook, without handing it the mobile key that identifies the @@ -1028,6 +1028,7 @@ public class LDClient { throttler = self.serviceFactory.makeThrottler(environmentReporter: environmentReporter) config = configuration + backgroundDelivery = BackgroundActivity(reason: "LaunchDarkly event delivery", logger: configuration.logger) context = startContext ?? LDContext() if config.autoEnvAttributes { diff --git a/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift b/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift index c31fed7c9..eda2bb543 100644 --- a/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift +++ b/LaunchDarkly/LaunchDarkly/ServiceObjects/BackgroundActivity.swift @@ -1,4 +1,5 @@ import Foundation +import OSLog /// Holds a background activity assertion so a backgrounded process is not suspended mid-delivery. /// @@ -12,6 +13,8 @@ import Foundation final class BackgroundActivity { typealias Work = (_ finished: @escaping () -> Void) -> Void typealias Assertion = (_ reason: String, _ whileHeld: @escaping Work) -> Void + /// What grants the assertion, so a test can refuse one, or expire one, without the system's cooperation. + typealias ExpiringActivity = (_ reason: String, _ whileGranted: @escaping (_ expired: Bool) -> Void) -> Void /// Upper bound for work that never calls `finished`. private static let maximumDuration: TimeInterval = 30 @@ -24,11 +27,15 @@ final class BackgroundActivity { /// The work to run again before the held assertion is released. Only to be used while holding `lock`. private var rerun: Work? - init(reason: String, assertion: @escaping Assertion = BackgroundActivity.holdProcessAssertion) { + init(reason: String, assertion: @escaping Assertion) { self.reason = reason self.assertion = assertion } + convenience init(reason: String, logger: OSLog) { + self.init(reason: reason, assertion: BackgroundActivity.processAssertion(logger: logger)) + } + /// Runs `work` under an assertion released when `work` calls `finished`, the system expires it, or /// `maximumDuration` passes. func run(_ work: @escaping Work) { @@ -63,25 +70,41 @@ final class BackgroundActivity { } } + /// An assertion from the process, over an injectable system call and cap so that a test can drive both. + /// /// Without assertions on this platform, `whileHeld` runs on the calling thread. - private static func holdProcessAssertion(reason: String, whileHeld: @escaping Work) { - #if os(iOS) || os(tvOS) || os(watchOS) - let finished = DispatchSemaphore(value: 0) - // The work does not wait on the assertion, and does not run inside it. The system refuses one outright under - // the very resource pressure this is here for, and a delivery it may suspend is better than no delivery. - ProcessInfo.processInfo.performExpiringActivity(withReason: reason) { expired in - guard !expired - else { - // The system taking the assertion back, or declining to give one. Nothing should stay blocked on it. - finished.signal() - return + static func processAssertion(logger: OSLog, + perform: @escaping ExpiringActivity = BackgroundActivity.performExpiringActivity, + maximumDuration: TimeInterval = BackgroundActivity.maximumDuration) -> Assertion { + { reason, whileHeld in + #if os(iOS) || os(tvOS) || os(watchOS) + let finished = DispatchSemaphore(value: 0) + // The work does not wait on the assertion, and does not run inside it. The system refuses one outright + // under the very resource pressure this is here for, and a delivery it may suspend is better than no + // delivery. + perform(reason) { expired in + guard !expired + else { + // The system taking the assertion back, or declining to give one. Nothing should stay blocked on + // it, and the work goes ahead either way. + os_log("%s unprotected by a background assertion: %s", + log: logger, type: .debug, String(describing: BackgroundActivity.self), reason) + finished.signal() + return + } + // The assertion lasts as long as this block, which runs on a system queue, not the caller's thread. + _ = finished.wait(timeout: .now() + maximumDuration) } - // The assertion lasts as long as this block, which runs on a system queue, not the caller's thread. - _ = finished.wait(timeout: .now() + maximumDuration) + whileHeld { finished.signal() } + #else + whileHeld {} + #endif } - whileHeld { finished.signal() } - #else - whileHeld {} + } + + private static func performExpiringActivity(_ reason: String, _ whileGranted: @escaping (Bool) -> Void) { + #if os(iOS) || os(tvOS) || os(watchOS) + ProcessInfo.processInfo.performExpiringActivity(withReason: reason, using: whileGranted) #endif } } diff --git a/LaunchDarkly/LaunchDarklyTests/ServiceObjects/BackgroundActivitySpec.swift b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/BackgroundActivitySpec.swift index a21687ccf..3975aa459 100644 --- a/LaunchDarkly/LaunchDarklyTests/ServiceObjects/BackgroundActivitySpec.swift +++ b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/BackgroundActivitySpec.swift @@ -76,5 +76,69 @@ final class BackgroundActivitySpec: QuickSpec { expect(released) == 2 } } + + describe("an assertion from the process") { + /// Stands in for `performExpiringActivity`, which answers on a system queue rather than the caller's. + /// + /// Signals `ended` once the block it was given returns, which is when the real system would take the + /// assertion back. + func system(granting: Bool, ended: DispatchSemaphore) -> BackgroundActivity.ExpiringActivity { + { _, whileGranted in + DispatchQueue.global().async { + whileGranted(!granting) + ended.signal() + } + } + } + + it("runs the work, and holds the assertion no longer, when the system grants one") { + let ended = DispatchSemaphore(value: 0) + let assertion = BackgroundActivity.processAssertion(logger: .disabled, + perform: system(granting: true, ended: ended), + maximumDuration: 30) + + var ran = false + assertion("test") { finished in + ran = true + finished() + } + + expect(ran) == true + // Held for as long as the work took rather than the full 30s, which this would otherwise wait out. + expect(ended.wait(timeout: .now() + 2)) == .success + } + + it("runs the work even where the system refuses an assertion outright") { + // Refusing calls the block once with `expired`, and never with `granted`. Work that ran inside that + // block would never run at all, and under resource pressure that is exactly when it is needed. + let ended = DispatchSemaphore(value: 0) + let assertion = BackgroundActivity.processAssertion(logger: .disabled, + perform: system(granting: false, ended: ended), + maximumDuration: 30) + + var ran = false + assertion("test") { finished in + ran = true + finished() + } + + expect(ran) == true + expect(ended.wait(timeout: .now() + 2)) == .success + } + + it("gives up the assertion at the maximum, where the work never finishes") { + let ended = DispatchSemaphore(value: 0) + let assertion = BackgroundActivity.processAssertion(logger: .disabled, + perform: system(granting: true, ended: ended), + maximumDuration: 0.3) + + let started = Date() + assertion("test") { _ in } + + expect(ended.wait(timeout: .now() + 3)) == .success + // Ended by the cap, rather than by the work or by never having been held. + expect(Date().timeIntervalSince(started)) > 0.2 + } + } } } From 17b141cf735d7a83eb367e461899844bd0121ee3 Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Mon, 12 Oct 2026 00:00:20 -0700 Subject: [PATCH 10/11] Run the process assertion specs only where the platform has assertions On macOS processAssertion runs the work inline and never asks the system, so the specs that wait for the system to end the assertion timed out there and failed test-swiftpm. macOS gets a spec for its own behavior instead. Co-authored-by: Cursor --- .../BackgroundActivitySpec.swift | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/LaunchDarkly/LaunchDarklyTests/ServiceObjects/BackgroundActivitySpec.swift b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/BackgroundActivitySpec.swift index 3975aa459..f3de67f69 100644 --- a/LaunchDarkly/LaunchDarklyTests/ServiceObjects/BackgroundActivitySpec.swift +++ b/LaunchDarkly/LaunchDarklyTests/ServiceObjects/BackgroundActivitySpec.swift @@ -77,6 +77,7 @@ final class BackgroundActivitySpec: QuickSpec { } } + #if os(iOS) || os(tvOS) || os(watchOS) describe("an assertion from the process") { /// Stands in for `performExpiringActivity`, which answers on a system queue rather than the caller's. /// @@ -140,5 +141,24 @@ final class BackgroundActivitySpec: QuickSpec { expect(Date().timeIntervalSince(started)) > 0.2 } } + #else + describe("an assertion from the process") { + it("runs the work on the calling thread without asking the system, which has no assertions here") { + var asked = false + let assertion = BackgroundActivity.processAssertion(logger: .disabled, + perform: { _, _ in asked = true }, + maximumDuration: 30) + + var ran = false + assertion("test") { finished in + ran = true + finished() + } + + expect(ran) == true + expect(asked) == false + } + } + #endif } } From 256432a5a9564f9ffd23ecf8681c43fb384d5a9e Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Wed, 21 Oct 2026 20:57:02 -0700 Subject: [PATCH 11/11] Mark the tier 2 public methods as experimental Co-authored-by: Cursor --- LaunchDarkly/LaunchDarkly/LDClientFlush.swift | 6 ++++++ LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/LaunchDarkly/LaunchDarkly/LDClientFlush.swift b/LaunchDarkly/LaunchDarkly/LDClientFlush.swift index d1bff28db..0bb11ba4a 100644 --- a/LaunchDarkly/LaunchDarkly/LDClientFlush.swift +++ b/LaunchDarkly/LaunchDarkly/LDClientFlush.swift @@ -26,6 +26,9 @@ extension LDClient { timeout, because a delivery always ends, by its own request timeouts if nothing else. This is the one to reach for from a Swift concurrency task, where blocking is not safe. + This method is not stable, and not subject to any backwards compatibility guarantees or semantic versioning. It is + experimental. + - parameter completion: Given `true` if LaunchDarkly accepted the events, or there were none to send, and `false` if the client is offline or closed, or any of the events were lost. See `flushAndWait` for what being lost covers. Runs once, on a background queue that is neither the caller's thread nor the one recorded events @@ -89,6 +92,9 @@ extension LDClient { 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. + This method is not stable, and not subject to any backwards compatibility guarantees or semantic versioning. It is + experimental. + - parameter timeout: How long to wait, in seconds. Zero or less does not wait, and so reports `false`. There is no upper limit: a delivery always ends, by its own request timeouts if nothing else, so a very long timeout waits for that. diff --git a/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift b/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift index 46d386954..0b7da3aa1 100644 --- a/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift +++ b/LaunchDarkly/LaunchDarkly/ObjectiveC/ObjcLDClient.swift @@ -548,6 +548,9 @@ public final class ObjcLDClient: NSObject { Like `flushAndWait`, but without blocking: the completion runs once every environment has answered. There is no timeout, because a delivery always ends, by its own request timeouts if nothing else. + This method is not stable, and not subject to any backwards compatibility guarantees or semantic versioning. It is + experimental. + - parameter completion: Given `YES` if LaunchDarkly accepted the events, or there were none to send, and `NO` if the client is offline or closed, or any of the events were lost. See `flushAndWait` for what being lost covers. Runs once, on a background queue that is not the caller's thread. @@ -572,6 +575,9 @@ public final class ObjcLDClient: NSObject { 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. + This method is not stable, and not subject to any backwards compatibility guarantees or semantic versioning. It is + experimental. + - parameter timeout: How long to wait, in seconds. Zero or less does not wait, and so reports `NO`. There is no upper limit: a delivery always ends, by its own request timeouts if nothing else, so a very long timeout waits for that.