-
Notifications
You must be signed in to change notification settings - Fork 331
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
v4: Fix main actor references in iOS 11-12 #4683
base: release/4.43.3
Are you sure you want to change the base?
Conversation
self.operationDispatcher.dispatchOnMainActor { | ||
self.operationDispatcher.dispatchAsyncOnMainThread { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: in this file I'm using dispatchAsyncOnMainThread
and not dispatchOnMainThread
. Not sure which one is best, haven't had time to dig in. But there is comment in line 307 where we're using dispatchAsyncOnMainThread
saying that it must be async to prevent deadlocks, so since I didn't have time to dig in deeper I just went with that for the whole file
public typealias PurchaseCompletedBlock = @MainActor @Sendable (StoreTransaction?, | ||
CustomerInfo?, | ||
PublicError?, | ||
Bool) -> Void | ||
// todo: main actor here | ||
public typealias PurchaseCompletedBlock = @Sendable (StoreTransaction?, | ||
CustomerInfo?, | ||
PublicError?, | ||
Bool) -> Void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the most concerning one, where we are effectively doing a public-facing change. But it's one that removes restrictions, rather than adding. I expect maybe it adds some Sendable warnings?
Then again... How in the world did we add this in a minor in the first place?
Having references to
@MainActor
crashes the SDK in runtime in iOS 11 and 12 in our SDK v4 when compiling with Xcode 16 (it's not an issue in Xcode 15).We've reported this to Apple as
FB16345033
.To solve this, we need to remove all references to
@MainActor
for iOS 11 and 12. Conceptually this makes sense - that should never have compiled in the first place, since iOS 11 and 12 do not support Swift Concurrency.But this is a fairly delicate open heart surgery, so we need to test it out thoroughly.