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

v4: Fix main actor references in iOS 11-12 #4683

Draft
wants to merge 4 commits into
base: release/4.43.3
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions Sources/Identity/CustomerInfoManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import Foundation

class CustomerInfoManager {

typealias CustomerInfoCompletion = @MainActor @Sendable (Result<CustomerInfo, BackendError>) -> Void
// todo: confirm removal of mainActor here
typealias CustomerInfoCompletion = @Sendable (Result<CustomerInfo, BackendError>) -> Void

private let offlineEntitlementsManager: OfflineEntitlementsManager
private let operationDispatcher: OperationDispatcher
Expand Down Expand Up @@ -89,7 +90,7 @@ class CustomerInfoManager {
}

if let completion = completion {
self.operationDispatcher.dispatchOnMainActor {
self.operationDispatcher.dispatchAsyncOnMainThread {
Comment on lines -92 to +93
Copy link
Member Author

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

completion(result)
}
}
Expand All @@ -114,7 +115,7 @@ class CustomerInfoManager {
}

if let completion = completion {
self.operationDispatcher.dispatchOnMainActor {
self.operationDispatcher.dispatchAsyncOnMainThread {
completion(.success(customerInfo))
}
}
Expand All @@ -128,7 +129,7 @@ class CustomerInfoManager {
) {
switch fetchPolicy {
case .fromCacheOnly:
self.operationDispatcher.dispatchOnMainActor {
self.operationDispatcher.dispatchAsyncOnMainThread {
completion?(
Result(self.cachedCustomerInfo(appUserID: appUserID), .missingCachedCustomerInfo())
)
Expand All @@ -149,7 +150,7 @@ class CustomerInfoManager {
Logger.debug(Strings.customerInfo.vending_cache)
if let completion = completion {
completionCalled = true
self.operationDispatcher.dispatchOnMainActor {
self.operationDispatcher.dispatchAsyncOnMainThread {
completion(.success(infoFromCache))
}
}
Expand All @@ -175,7 +176,7 @@ class CustomerInfoManager {
if let infoFromCache = infoFromCache, !isCacheStale {
Logger.debug(Strings.customerInfo.vending_cache)
if let completion = completion {
self.operationDispatcher.dispatchOnMainActor {
self.operationDispatcher.dispatchAsyncOnMainThread {
completion(.success(infoFromCache))
}
}
Expand Down
2 changes: 2 additions & 0 deletions Sources/Misc/Concurrency/OperationDispatcher.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class OperationDispatcher {
self.mainQueue.async(execute: block)
}

// todo: confirm all references to this have OS checks
func dispatchOnMainActor(_ block: @MainActor @escaping @Sendable () -> Void) {
Self.dispatchOnMainActor(block)
}
Expand Down Expand Up @@ -79,6 +80,7 @@ class OperationDispatcher {

extension OperationDispatcher {

// todo: main actor needs OS check
static func dispatchOnMainActor(_ block: @MainActor @escaping @Sendable () -> Void) {
if #available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) {
Task<Void, Never> { @MainActor in
Expand Down
10 changes: 5 additions & 5 deletions Sources/Misc/SystemInfo.swift
Original file line number Diff line number Diff line change
Expand Up @@ -149,15 +149,15 @@ class SystemInfo {
/// Asynchronous API if caller can't ensure that it's invoked in the `@MainActor`
/// - Seealso: `isApplicationBackgrounded`
func isApplicationBackgrounded(completion: @escaping @Sendable (Bool) -> Void) {
self.operationDispatcher.dispatchOnMainActor {
self.operationDispatcher.dispatchOnMainThread {
completion(self.isApplicationBackgrounded)
}
}

/// Synchronous API for callers in `@MainActor`.
/// - Seealso: `isApplicationBackgrounded(completion:)`
@MainActor
var isApplicationBackgrounded: Bool {
// todo: main actor here
private var isApplicationBackgrounded: Bool {
#if os(iOS) || os(tvOS) || VISION_OS
return self.isApplicationBackgroundedIOSAndTVOS
#elseif os(macOS)
Expand Down Expand Up @@ -274,7 +274,7 @@ private extension SystemInfo {
// iOS/tvOS App extensions can't access UIApplication.sharedApplication, and will fail to compile if any calls to
// it are made. There are no pre-processor macros available to check if the code is running in an app extension,
// so we check if we're running in an app extension at runtime, and if not, we use KVC to call sharedApplication.
@MainActor
// todo: main actor here
var isApplicationBackgroundedIOSAndTVOS: Bool {
if self.isAppExtension {
return true
Expand All @@ -286,7 +286,7 @@ private extension SystemInfo {

#elseif os(watchOS)

@MainActor
// todo: main actor here
var isApplicationBackgroundedWatchOS: Bool {
var isSingleTargetApplication: Bool {
return Bundle.main.infoDictionary?.keys.contains("WKApplication") == true
Expand Down
8 changes: 5 additions & 3 deletions Sources/OfflineEntitlements/OfflineEntitlementsManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ class OfflineEntitlementsManager {
self.systemInfo = systemInfo
}

// todo: main actor here
func updateProductsEntitlementsCacheIfStale(
isAppBackgrounded: Bool,
completion: (@MainActor @Sendable (Result<(), Error>) -> Void)?
completion: (@Sendable (Result<(), Error>) -> Void)?
) {
guard #available(iOS 15.0, tvOS 15.0, watchOS 8.0, macOS 12.0, *),
!self.systemInfo.observerMode,
Expand Down Expand Up @@ -103,12 +104,13 @@ private extension OfflineEntitlementsManager {

private extension OfflineEntitlementsManager {

// todo: main actor here
func dispatchCompletionOnMainThreadIfPossible<Value, Error: Swift.Error>(
_ completion: (@MainActor @Sendable (Result<Value, Error>) -> Void)?,
_ completion: (@Sendable (Result<Value, Error>) -> Void)?,
result: Result<Value, Error>
) {
if let completion = completion {
self.operationDispatcher.dispatchOnMainActor {
self.operationDispatcher.dispatchOnMainThread {
completion(result)
}
}
Expand Down
20 changes: 13 additions & 7 deletions Sources/Purchasing/OfferingsManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@ class OfferingsManager {
self.productsManager = productsManager
}

// todo: main actor here
func offerings(
appUserID: String,
fetchPolicy: FetchPolicy = .default,
fetchCurrent: Bool = false,
completion: (@MainActor @Sendable (Result<Offerings, Error>) -> Void)?
completion: (@Sendable (Result<Offerings, Error>) -> Void)?
) {
guard !fetchCurrent else {
self.fetchFromNetwork(appUserID: appUserID, fetchPolicy: fetchPolicy, completion: completion)
Expand Down Expand Up @@ -72,11 +73,12 @@ class OfferingsManager {
return self.deviceCache.cachedOfferings
}

// todo: main actor here
func updateOfferingsCache(
appUserID: String,
isAppBackgrounded: Bool,
fetchPolicy: FetchPolicy = .default,
completion: (@MainActor @Sendable (Result<Offerings, Error>) -> Void)?
completion: (@Sendable (Result<Offerings, Error>) -> Void)?
) {
self.backend.offerings.getOfferings(appUserID: appUserID, isAppBackgrounded: isAppBackgrounded) { result in
switch result {
Expand Down Expand Up @@ -132,10 +134,11 @@ class OfferingsManager {

private extension OfferingsManager {

// todo: main actor here
func fetchFromNetwork(
appUserID: String,
fetchPolicy: FetchPolicy = .default,
completion: (@MainActor @Sendable (Result<Offerings, Error>) -> Void)?
completion: (@Sendable (Result<Offerings, Error>) -> Void)?
) {
Logger.debug(Strings.offering.no_cached_offerings_fetching_from_network)

Expand Down Expand Up @@ -225,11 +228,12 @@ private extension OfferingsManager {
}
}

// todo: main actor here
func handleOfferingsBackendResult(
with response: OfferingsResponse,
appUserID: String,
fetchPolicy: FetchPolicy,
completion: (@MainActor @Sendable (Result<Offerings, Error>) -> Void)?
completion: (@Sendable (Result<Offerings, Error>) -> Void)?
) {
self.createOfferings(from: response, fetchPolicy: fetchPolicy) { result in
switch result {
Expand All @@ -255,21 +259,23 @@ private extension OfferingsManager {
}
}

// todo: main actor here
func handleOfferingsUpdateError(
_ error: Error,
completion: (@MainActor @Sendable (Result<Offerings, Error>) -> Void)?
completion: (@Sendable (Result<Offerings, Error>) -> Void)?
) {
Logger.appleError(Strings.offering.fetching_offerings_error(error: error,
underlyingError: error.underlyingError))
self.dispatchCompletionOnMainThreadIfPossible(completion, value: .failure(error))
}

// todo: verify main actor replacement here
func dispatchCompletionOnMainThreadIfPossible<T>(
_ completion: (@MainActor @Sendable (T) -> Void)?,
_ completion: (@Sendable (T) -> Void)?,
value: T
) {
if let completion = completion {
self.operationDispatcher.dispatchOnMainActor {
self.operationDispatcher.dispatchOnMainThread {
completion(value)
}
}
Expand Down
9 changes: 5 additions & 4 deletions Sources/Purchasing/Purchases/Purchases.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@ public typealias PurchaseResultData = (transaction: StoreTransaction?,
/**
Completion block for ``Purchases/purchase(product:completion:)``
*/
public typealias PurchaseCompletedBlock = @MainActor @Sendable (StoreTransaction?,
CustomerInfo?,
PublicError?,
Bool) -> Void
// todo: main actor here
public typealias PurchaseCompletedBlock = @Sendable (StoreTransaction?,
CustomerInfo?,
PublicError?,
Bool) -> Void
Comment on lines -36 to +40
Copy link
Member Author

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?


/**
Block for starting purchases in ``PurchasesDelegate/purchases(_:readyForPromotedProduct:purchase:)``
Expand Down
6 changes: 4 additions & 2 deletions Sources/Purchasing/Purchases/PurchasesOrchestrator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -374,8 +374,9 @@ final class PurchasesOrchestrator {
* we wouldn't be able to notify of the purchase result.
*/

// todo: verify actor replacement here
guard let productIdentifier = payment.extractProductIdentifier() else {
self.operationDispatcher.dispatchOnMainActor {
self.operationDispatcher.dispatchOnMainThread {
completion(
nil,
nil,
Expand Down Expand Up @@ -1291,8 +1292,9 @@ private extension PurchasesOrchestrator {
}
}

// todo: verify actor replacement here
func handleTestProduct(_ completion: @escaping PurchaseCompletedBlock) {
self.operationDispatcher.dispatchOnMainActor {
self.operationDispatcher.dispatchOnMainThread {
completion(
nil,
nil,
Expand Down
9 changes: 6 additions & 3 deletions Sources/Purchasing/Purchases/TransactionPoster.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,10 @@ protocol TransactionPosterType: AnyObject, Sendable {
/// Finishes the transaction if not in observer mode.
/// - Note: `handlePurchasedTransaction` calls this automatically,
/// this is only required for failed transactions.
// todo: main actor here
func finishTransactionIfNeeded(
_ transaction: StoreTransactionType,
completion: @escaping @Sendable @MainActor () -> Void
completion: @escaping @Sendable () -> Void
)

}
Expand Down Expand Up @@ -121,13 +122,14 @@ final class TransactionPoster: TransactionPosterType {
}
}

// todo: main actor here
func finishTransactionIfNeeded(
_ transaction: StoreTransactionType,
completion: @escaping @Sendable @MainActor () -> Void
completion: @escaping @Sendable () -> Void
) {
@Sendable
func complete() {
self.operationDispatcher.dispatchOnMainActor(completion)
self.operationDispatcher.dispatchOnMainThread(completion)
}

guard self.finishTransactions else {
Expand Down Expand Up @@ -330,6 +332,7 @@ extension TransactionPoster {

private extension TransactionPoster {

// todo: verify actor replacement ehre
func product(with identifier: String, completion: @escaping (StoreProduct?) -> Void) {
self.productsManager.products(withIdentifiers: [identifier]) { products in
self.operationDispatcher.dispatchOnMainThread {
Expand Down
8 changes: 6 additions & 2 deletions Sources/Purchasing/StoreKit1/StoreKitRequestFetcher.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ class StoreKitRequestFetcher: NSObject {

private let requestFactory: ReceiptRefreshRequestFactory
private var receiptRefreshRequest: SKRequest?
private var receiptRefreshCompletionHandlers: [@MainActor @Sendable () -> Void]
// todo: main actor here
private var receiptRefreshCompletionHandlers: [@Sendable () -> Void]

private let operationDispatcher: OperationDispatcher

init(requestFactory: ReceiptRefreshRequestFactory = ReceiptRefreshRequestFactory(),
Expand All @@ -38,6 +40,7 @@ class StoreKitRequestFetcher: NSObject {
self.receiptRefreshCompletionHandlers = []
}

// todo: main actor here
func fetchReceiptData(_ completion: @MainActor @Sendable @escaping () -> Void) {
self.operationDispatcher.dispatchOnWorkerThread {
self.receiptRefreshCompletionHandlers.append(completion)
Expand Down Expand Up @@ -93,8 +96,9 @@ private extension StoreKitRequestFetcher {
let completionHandlers = self.receiptRefreshCompletionHandlers
self.receiptRefreshCompletionHandlers = []

// todo: verify main actor replacement here
for handler in completionHandlers {
self.operationDispatcher.dispatchOnMainActor {
self.operationDispatcher.dispatchOnMainThread {
handler()
}
}
Expand Down