Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Completion invoked twice #428

Open
macdrevx opened this issue Feb 12, 2025 · 6 comments
Open

Completion invoked twice #428

macdrevx opened this issue Feb 12, 2025 · 6 comments

Comments

@macdrevx
Copy link

We wrap LDClient.start in withCheckedContinuation and we have a crash stemming from the continuation being invoked twice. Looking at the implementation, I'm pretty sure there's a race condition here that permits the completion block to be invoked more than once:

let startTime = Date().timeIntervalSince1970
start(serviceFactory: serviceFactory, config: config, context: context) {
internalCompletedQueue.async {
if startTime + startWaitSeconds > Date().timeIntervalSince1970 && completed {
completed = false
completion?(completed)
}
}
}
internalCompletedQueue.asyncAfter(deadline: .now() + startWaitSeconds) {
if completed {
completion?(completed)
}
}

@macdrevx
Copy link
Author

It seems like the assumption here is that the closure passed to internalCompletedQueue.async on line 838 will always be invoked before the closure passed to internalCompletedQueue.asyncAfter(deadline:) on line 845.

However, if (1) the closure passed to internalCompletedQueue.asyncAfter(deadline:) runs first and (2) the closure passed to internalCompletedQueue.async runs while startTime + startWaitSeconds > Date().timeIntervalSince1970 is still true, then the completion closure will be invoked twice.

In our case, we're passing 10 for startWaitSeconds.

@tanderson-ld
Copy link
Contributor

Thank you @macdrevx, taking a look.

@tanderson-ld
Copy link
Contributor

We have merged a fix in #429 and will be releasing that shortly. Thank you again for investigating and helping us hone in on that spot of code. Which version of the SDK are you using?

@macdrevx
Copy link
Author

Thanks! We're on 9.12.0

@macdrevx
Copy link
Author

Also, I left a small suggestion on that pr: https://github.com/launchdarkly/ios-client-sdk/pull/429/files#r1953291290

@tanderson-ld
Copy link
Contributor

Your suggestions are absolutely valid. I thought about that while making the tweak, but wanted to minimize impact risk. I'll keep those in mind if I touch that code in the future.

9.12.3 was released with the fix and should be propagating through the package distribution systems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants