Always call readyForPromotedProduct on main thread/actor #4613
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@RCGitBot please test |
|
@RCGitBot please test |
|
@RCGitBot please test |
|
@RCGitBot please test |
12 tasks
|
@RCGitBot please test |
|
@RCGitBot please test |
fire-at-will
changed the title
|
@RCGitBot please test |
|
@RCGitBot please test |
| switch self.systemInfo.storeKitVersion.effectiveVersion { | ||
| case .storeKit1: | ||
| // Calling the delegate method on the main actor causes test failures on iOS 14-16, so instead | ||
| // we dispatch to the main thread, which doesn't cause the failures. |
There was a problem hiding this comment.
yeah i think this is good practice anyway, specially if we're calling on the main actor in sk2
MarkVillacampa
approved these changes
Feb 3, 2025
This reverts commit 37431f2.
This was referenced Feb 5, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Potential fix for #4582. While I haven't been able to reproduce the crash, a user on the issue has reported that this fixes the crash.
Description
#4582 describes an occasional crash that occurs when the SDK calls the PurchaseDelegate's readyForPromotedProduct function. The provided stacktraces show a threading issue occurring in both the SK1 & SK2 flows. Our working theory is that the calling code may expect
readyForPromotedProductto be called on the main thread/actor, and it is being called on a background thread.This PR ensures that
readyForPromotedProductis always called on the Main Actor (for SK2) and main thread (for SK1).A user on the GH Issue has reported that this PR resolves the crash for them.
SK1 & SK2 Differences
#4584 attempted to address this issue by always calling
readyForPromotedProducton the main actor, but we found that this caused very consistent, but still flaky, test failures in SK1 on iOS 14-16. Out of an abundance of caution, this was reverted in #4599.This PR calls
readyForPromotedProducton the main thread instead of the main actor for SK1, which has addressed these test failures.Testing
While I've been unable to reproduce the crash, I've tested the fix with both StoreKit 1 & 2 on iOS 17-18. I've been unable to test in iOS 15-16 because Xcode doesn't allow you to simulate Purchase Intents in simulators on iOS <17, and all of my physical devices are on iOS 18. That said, all automated tests pass on all iOS versions.