Skip to content

Commit f76f15e

Browse files
abelonogov-ldabelonogov-ld
authored
feat!: Timeout executor to fix race conditions on timeout (#457)
**Requirements** - [x ] I have added test coverage for new or changed functionality - [ x] I have followed the repository's [pull request submission guidelines](../blob/v9/CONTRIBUTING.md#submitting-pull-requests) - [ x] I have validated my changes against all supported platform versions **Describe the solution you've provided** Implemented Timeout executor to fix race condition, when completions could called twice, by providing stable implementation of Timeout **Describe alternatives you've considered** Provide a clear and concise description of any alternative solutions or features you've considered. **Additional context** Add any other context about the pull request here. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Introduces a reusable TimeoutExecutor and uses it to streamline timeout behavior in LDClient identify and start, with accompanying unit tests and project wiring. > > - **Core**: > - **Utility**: Add `ServiceObjects/TimeoutExecutor.swift` to run async operations with a timeout fallback and ensure single completion. > - **Refactor**: Replace custom timeout logic in `LDClient.identify(context:timeout:useCache:completion:)` and `LDClient.start(..., startWaitSeconds:, ...)` with `TimeoutExecutor`. > - **Tests**: > - Add `LaunchDarklyTests/ServiceObjects/TimeoutExecutorSpec.swift` covering result-before-timeout, timeout fallback, race conditions, queue execution, nil completion, and late-result ignored. > - **Project**: > - Wire new sources/tests into all targets; set team id for tests. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 6630979. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
2 parents 068a422 + 6630979 commit f76f15e

File tree

4 files changed

+282
-31
lines changed

4 files changed

+282
-31
lines changed

‎LaunchDarkly.xcodeproj/project.pbxproj‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@
3838
3D3AB9462A4F16FE003AECF1 /* ReportingConsts.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3D3AB9422A4F16FE003AECF1 /* ReportingConsts.swift */; };
3939
3D3AB9482A570F3A003AECF1 /* ModifierSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3D3AB9472A570F3A003AECF1 /* ModifierSpec.swift */; };
4040
3D9A12582A73236800698B8D /* UtilSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3D9A12572A73236800698B8D /* UtilSpec.swift */; };
41+
50EE85C22EA0487F007CC662 /* TimeoutExecutor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50EE85C12EA0487F007CC662 /* TimeoutExecutor.swift */; };
42+
50EE85C32EA0487F007CC662 /* TimeoutExecutor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50EE85C12EA0487F007CC662 /* TimeoutExecutor.swift */; };
43+
50EE85C42EA0487F007CC662 /* TimeoutExecutor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50EE85C12EA0487F007CC662 /* TimeoutExecutor.swift */; };
44+
50EE85C52EA0487F007CC662 /* TimeoutExecutor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50EE85C12EA0487F007CC662 /* TimeoutExecutor.swift */; };
45+
50EE85C72EA0749C007CC662 /* TimeoutExecutorSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = 50EE85C62EA0749C007CC662 /* TimeoutExecutorSpec.swift */; };
4146
830BF933202D188E006DF9B1 /* HTTPURLRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 830BF932202D188E006DF9B1 /* HTTPURLRequest.swift */; };
4247
830DB3AC22380A3E00D65D25 /* HTTPHeadersSpec.swift in Sources */ = {isa = PBXBuildFile; fileRef = 830DB3AB22380A3E00D65D25 /* HTTPHeadersSpec.swift */; };
4348
830DB3AE2239B54900D65D25 /* URLResponse.swift in Sources */ = {isa = PBXBuildFile; fileRef = 830DB3AD2239B54900D65D25 /* URLResponse.swift */; };
@@ -429,6 +434,8 @@
429434
3D3AB9422A4F16FE003AECF1 /* ReportingConsts.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReportingConsts.swift; sourceTree = "<group>"; };
430435
3D3AB9472A570F3A003AECF1 /* ModifierSpec.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ModifierSpec.swift; sourceTree = "<group>"; };
431436
3D9A12572A73236800698B8D /* UtilSpec.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UtilSpec.swift; sourceTree = "<group>"; };
437+
50EE85C12EA0487F007CC662 /* TimeoutExecutor.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TimeoutExecutor.swift; sourceTree = "<group>"; };
438+
50EE85C62EA0749C007CC662 /* TimeoutExecutorSpec.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TimeoutExecutorSpec.swift; sourceTree = "<group>"; };
432439
830BF932202D188E006DF9B1 /* HTTPURLRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HTTPURLRequest.swift; sourceTree = "<group>"; };
433440
830DB3AB22380A3E00D65D25 /* HTTPHeadersSpec.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HTTPHeadersSpec.swift; sourceTree = "<group>"; };
434441
830DB3AD2239B54900D65D25 /* URLResponse.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = URLResponse.swift; sourceTree = "<group>"; };
@@ -629,6 +636,7 @@
629636
831D8B751F72A48900ED65E8 /* ServiceObjects */ = {
630637
isa = PBXGroup;
631638
children = (
639+
50EE85C62EA0749C007CC662 /* TimeoutExecutorSpec.swift */,
632640
A3047D5B2A606A0000F568E0 /* EnvironmentReporting */,
633641
B46F344025E6DB7D0078D45F /* DiagnosticReporterSpec.swift */,
634642
83CFE7CD1F7AD81D0010544E /* EventReporterSpec.swift */,
@@ -887,6 +895,7 @@
887895
83FEF8D91F2666BF001CF12C /* ServiceObjects */ = {
888896
isa = PBXGroup;
889897
children = (
898+
50EE85C12EA0487F007CC662 /* TimeoutExecutor.swift */,
890899
A3A8BCD12B7EAA89009A77E4 /* SheddingQueue.swift */,
891900
A358D6CF2A4DD45000270C60 /* EnvironmentReporting */,
892901
8354AC742243168800CDE602 /* Cache */,
@@ -1355,6 +1364,7 @@
13551364
3D3AB9462A4F16FE003AECF1 /* ReportingConsts.swift in Sources */,
13561365
831188522113ADF700D77CB5 /* KeyedValueCache.swift in Sources */,
13571366
831188582113AE0F00D77CB5 /* EventReporter.swift in Sources */,
1367+
50EE85C42EA0487F007CC662 /* TimeoutExecutor.swift in Sources */,
13581368
A358D6F52A4DEB4C00270C60 /* EnvironmentReporterBuilder.swift in Sources */,
13591369
8311885D2113AE2500D77CB5 /* DarklyService.swift in Sources */,
13601370
831188692113AE5900D77CB5 /* ObjcLDConfig.swift in Sources */,
@@ -1481,6 +1491,7 @@
14811491
A3A8BCD42B7EAA89009A77E4 /* SheddingQueue.swift in Sources */,
14821492
B4C9D43A2489E20A004A9B03 /* DiagnosticReporter.swift in Sources */,
14831493
831EF36A20655E730001C643 /* ObjcLDChangedFlag.swift in Sources */,
1494+
50EE85C22EA0487F007CC662 /* TimeoutExecutor.swift in Sources */,
14841495
);
14851496
runOnlyForDeploymentPostprocessing = 0;
14861497
};
@@ -1508,6 +1519,7 @@
15081519
3D3AB9432A4F16FE003AECF1 /* ReportingConsts.swift in Sources */,
15091520
A31088172837DC0400184942 /* Reference.swift in Sources */,
15101521
8354EFE21F26380700C05156 /* Event.swift in Sources */,
1522+
50EE85C52EA0487F007CC662 /* TimeoutExecutor.swift in Sources */,
15111523
A358D6F22A4DEB4C00270C60 /* EnvironmentReporterBuilder.swift in Sources */,
15121524
C408884923033B7500420721 /* ConnectionInformation.swift in Sources */,
15131525
831D8B721F71D3E700ED65E8 /* DarklyService.swift in Sources */,
@@ -1606,6 +1618,7 @@
16061618
838AB53F1F72A7D5006F03F5 /* FlagSynchronizerSpec.swift in Sources */,
16071619
A3FFE1132B7D4BA2009EF93F /* LDValueDecoderSpec.swift in Sources */,
16081620
A3570F5A28527B8200CF241A /* LDContextCodableSpec.swift in Sources */,
1621+
50EE85C72EA0749C007CC662 /* TimeoutExecutorSpec.swift in Sources */,
16091622
837406D421F760640087B22B /* LDTimerSpec.swift in Sources */,
16101623
832307A61F7D8D720029815A /* URLRequestSpec.swift in Sources */,
16111624
A3BA7D022BD192240000DB28 /* LDClientHookSpec.swift in Sources */,
@@ -1639,6 +1652,7 @@
16391652
3D3AB9442A4F16FE003AECF1 /* ReportingConsts.swift in Sources */,
16401653
83D9EC7E2062DEAB004D7FA6 /* FlagChangeObserver.swift in Sources */,
16411654
83D9EC7F2062DEAB004D7FA6 /* FlagsUnchangedObserver.swift in Sources */,
1655+
50EE85C32EA0487F007CC662 /* TimeoutExecutor.swift in Sources */,
16421656
A358D6F32A4DEB4C00270C60 /* EnvironmentReporterBuilder.swift in Sources */,
16431657
83D9EC802062DEAB004D7FA6 /* Event.swift in Sources */,
16441658
83D9EC822062DEAB004D7FA6 /* ClientServiceFactory.swift in Sources */,
@@ -2019,6 +2033,7 @@
20192033
isa = XCBuildConfiguration;
20202034
buildSettings = {
20212035
ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = "$(inherited)";
2036+
DEVELOPMENT_TEAM = 53D32B66PT;
20222037
INFOPLIST_FILE = "$(PROJECT_DIR)/LaunchDarkly/LaunchDarklyTests/Info.plist";
20232038
IPHONEOS_DEPLOYMENT_TARGET = 13.0;
20242039
PRODUCT_BUNDLE_IDENTIFIER = com.launchdarkly.DarklyTests;
@@ -2030,6 +2045,7 @@
20302045
isa = XCBuildConfiguration;
20312046
buildSettings = {
20322047
ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = "$(inherited)";
2048+
DEVELOPMENT_TEAM = 53D32B66PT;
20332049
INFOPLIST_FILE = "$(PROJECT_DIR)/LaunchDarkly/LaunchDarklyTests/Info.plist";
20342050
IPHONEOS_DEPLOYMENT_TARGET = 13.0;
20352051
PRODUCT_BUNDLE_IDENTIFIER = com.launchdarkly.DarklyTests;

‎LaunchDarkly/LaunchDarkly/LDClient.swift‎

Lines changed: 23 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -384,22 +384,18 @@ public class LDClient {
384384
if timeout > LDClient.longTimeoutInterval {
385385
os_log("%s LDClient.identify 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)
386386
}
387-
388-
var cancel = false
389-
390-
DispatchQueue.global().asyncAfter(deadline: .now() + timeout) {
391-
guard !cancel else { return }
392-
393-
cancel = true
394-
completion(.timeout)
395-
}
396-
397-
identify(context: context, useCache: useCache) { result in
398-
guard !cancel else { return }
399-
400-
cancel = true
401-
completion(result)
402-
}
387+
388+
TimeoutExecutor.run(
389+
timeout: timeout,
390+
queue: .global(),
391+
operation: { done in
392+
self.identify(context: context, useCache: useCache) { result in
393+
done(result)
394+
}
395+
},
396+
timeoutValue: .timeout,
397+
completion: completion
398+
)
403399
}
404400

405401
func internalIdentify(newContext: LDContext, useCache: IdentifyCacheUsage, completion: (() -> Void)? = nil) {
@@ -854,27 +850,23 @@ public class LDClient {
854850
}
855851

856852
static func start(serviceFactory: ClientServiceCreating?, config: LDConfig, context: LDContext? = nil, startWaitSeconds: TimeInterval, completion: ((_ timedOut: Bool) -> Void)? = nil) {
857-
var completed = false
858853
let internalCompletedQueue: DispatchQueue = DispatchQueue(label: "TimeOutQueue")
859854
if !config.startOnline {
860855
start(serviceFactory: serviceFactory, config: config, context: context)
856+
// Consider to wrap this into internalCompletedQueue to make completion return always consistent
861857
completion?(true) // offline is considered a short circuited timed out case
862858
} else {
863-
let startTime = Date().timeIntervalSince1970
864-
start(serviceFactory: serviceFactory, config: config, context: context) {
865-
internalCompletedQueue.async {
866-
if startTime + startWaitSeconds > Date().timeIntervalSince1970 && !completed {
867-
completed = true
868-
completion?(false) // false for not timedOut
859+
TimeoutExecutor.run(
860+
timeout: startWaitSeconds,
861+
queue: internalCompletedQueue,
862+
operation: { done in
863+
start(serviceFactory: serviceFactory, config: config, context: context) {
864+
done(false)
869865
}
870-
}
871-
}
872-
internalCompletedQueue.asyncAfter(deadline: .now() + startWaitSeconds) {
873-
if !completed {
874-
completed = true
875-
completion?(true) // true for timedOut
876-
}
877-
}
866+
},
867+
timeoutValue: true,
868+
completion: completion
869+
)
878870
}
879871
}
880872

‎LaunchDarkly/LaunchDarkly/ServiceObjects/TimeoutExecutor.swift‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import Foundation
2+
3+
/// A lightweight utility for executing asynchronous operations with a timeout fallback.
4+
///
5+
/// `TimeoutExecutor` guarantees that the provided `completion` closure is called **exactly once**,
6+
/// either with the result of the asynchronous operation or with a timeout value if the operation
7+
/// does not complete in time.
8+
///
9+
/// ### Typical Usage
10+
/// ```swift
11+
/// TimeoutExecutor.run(
12+
/// timeout: 2.0,
13+
/// queue: .main,
14+
/// operation: { done in
15+
/// service.action() {
16+
/// done("Success")
17+
/// }
18+
/// },
19+
/// timeoutValue: "Timeout",
20+
/// completion: { result in
21+
/// print("Result:", result)
22+
/// }
23+
/// )
24+
/// ```
25+
///
26+
final class TimeoutExecutor {
27+
private init() {}
28+
29+
static func run<T>(
30+
timeout: TimeInterval,
31+
queue: DispatchQueue = .global(),
32+
operation: (@escaping (T) -> Void) -> Void,
33+
timeoutValue: @autoclosure @escaping () -> T,
34+
completion: ((T) -> Void)?
35+
) {
36+
guard let completion = completion else {
37+
operation { _ in
38+
/* ignore result */
39+
}
40+
return
41+
}
42+
43+
let lockQueue = DispatchQueue(label: "launchdarkly.timeout.executor.lock")
44+
var finished = false
45+
46+
// Start the user operation
47+
operation { value in
48+
var shouldCall = false
49+
lockQueue.sync {
50+
if !finished {
51+
finished = true
52+
shouldCall = true
53+
}
54+
}
55+
56+
if shouldCall {
57+
queue.async { completion(value) }
58+
}
59+
}
60+
61+
// Timeout fallback
62+
queue.asyncAfter(deadline: .now() + timeout) {
63+
var shouldCall = false
64+
lockQueue.sync {
65+
if !finished {
66+
finished = true
67+
shouldCall = true
68+
}
69+
}
70+
71+
if shouldCall {
72+
completion(timeoutValue())
73+
}
74+
}
75+
}
76+
}

0 commit comments

Comments
 (0)