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

[DO NOT MERGE] always call readyForPromotedProduct on main thread/actor #4613

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

fire-at-will
Copy link
Contributor

@fire-at-will fire-at-will commented Dec 19, 2024

Motivation

Potential fix for #4582

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 readyForPromotedProduct to be called on the main thread/actor, and it is being called on a background thread.

This PR ensures that readyForPromotedProduct is always called on the Main Actor (for SK2) and main thread (for SK1).

SK1 & SK2 Differences

#4584 attempted to address this issue by always calling readyForPromotedProduct on 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 readyForPromotedProduct on the main thread instead of the main actor for SK1, which seems to address these test failures. The SK1 tests have passed each time I've run them on this PR.

@fire-at-will
Copy link
Contributor Author

@RCGitBot please test

@fire-at-will fire-at-will added the pr:fix A bug fix label Dec 20, 2024
@fire-at-will
Copy link
Contributor Author

@RCGitBot please test

@fire-at-will
Copy link
Contributor Author

@RCGitBot please test

@fire-at-will
Copy link
Contributor Author

@RCGitBot please test

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

Successfully merging this pull request may close these issues.

1 participant