From 0a4236c694311aaa0e6d030177c8c2b5a2767f50 Mon Sep 17 00:00:00 2001 From: Arthur Geron <3487334+arthurgeron@users.noreply.github.com> Date: Tue, 11 Jun 2024 23:39:30 -0300 Subject: [PATCH] [Fix] - Store Kit 1 race condition (#2604) **Fixes:** #1696, #2372 Similar to #2518, this fixes race condition crashes on `validProducts` and `productsRequest` in `RNIapIos.swift` when using Store Kit 1. ## React Native IAP ### What was done: - **Created `ThreadSafe` class:** This now wraps `validProducts`. - **Created `LatestPromiseKeeper`:** - Maintains only the latest promise to fix racing conditions when multiple `productsRequest` are made in parallel. - Resolves only one promise at any given time (for `productsRequest`), eliminating the possibility of racing conditions or execution exceptions while storing the resolved data. - Discards older promises if more than one is made, returning a rejection for the discarded promises. ## IAPExample (iOS) - Fixed target iOS deployment being loaded from an undefined variable. It now fetches the project's target iOS from the Xcode project and applies it during pod install. - Updated GitHub Action to use Node.js 20. - Changed GitHub Action runner from M1 architecture to Intel-based macOS (fixes crash during build). --- .github/workflows/ci-example-ios.yml | 32 +++++++++--------- .gitignore | 1 + IapExample/README.md | 2 +- IapExample/ios/Podfile | 19 ++++++++++- ios/LatestPromiseKeeper.swift | 47 ++++++++++++++++++++++++++ ios/RNIapIos.swift | 40 +++++++++------------- ios/RNIapIos.xcodeproj/project.pbxproj | 20 +++++++---- ios/ThreadSafe.swift | 18 ++++++++++ 8 files changed, 130 insertions(+), 49 deletions(-) create mode 100644 ios/LatestPromiseKeeper.swift create mode 100644 ios/ThreadSafe.swift diff --git a/.github/workflows/ci-example-ios.yml b/.github/workflows/ci-example-ios.yml index 377d1124e..3fbe73394 100644 --- a/.github/workflows/ci-example-ios.yml +++ b/.github/workflows/ci-example-ios.yml @@ -19,14 +19,16 @@ on: jobs: build_ios_example: - runs-on: macos-latest + runs-on: macos-13 + env: + NO_FLIPPER: 1 steps: - name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Setup Node.js - uses: actions/setup-node@v3 + uses: actions/setup-node@v4 with: cache: 'yarn' @@ -40,6 +42,8 @@ jobs: - name: Restore buildcache uses: mikehardy/buildcache-action@v2 continue-on-error: true + with: + cache_key: ${{ runner.os }}-buildcache-${{ hashFiles('**/Podfile.lock') }}-${{ hashFiles('**/Podfile')}} - name: Setup Ruby (bundle) uses: ruby/setup-ruby@v1 @@ -58,13 +62,13 @@ jobs: run: git diff --exit-code HEAD '*.swift' - name: Restore Pods cache - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: | IapExample/ios/Pods ~/Library/Caches/CocoaPods ~/.cocoapods - key: ${{ runner.os }}-pods-${{ hashFiles('**/Podfile.lock') }} + key: ${{ runner.os }}-pods-${{ hashFiles('**/Podfile.lock')}}-${{ hashFiles('**/Podfile')}} restore-keys: ${{ runner.os }}-pods- - name: Install Pods @@ -76,14 +80,10 @@ jobs: working-directory: IapExample/ios - name: Build App - run: "set -o pipefail && xcodebuild \ - CC=clang CPLUSPLUS=clang++ LD=clang LDPLUSPLUS=clang++ \ - -derivedDataPath build -UseModernBuildSystem=YES \ - -workspace IapExample.xcworkspace \ - -scheme IapExample \ - -sdk iphonesimulator \ - -configuration Debug \ - -destination 'platform=iOS Simulator,name=iPhone 11 Pro' \ - build \ - CODE_SIGNING_ALLOWED=NO | xcpretty" - working-directory: IapExample/ios + uses: sersoft-gmbh/xcodebuild-action@v3 + with: + workspace: IapExample/ios/IapExample.xcworkspace + scheme: IapExample + sdk: iphonesimulator + destination: 'platform=iOS Simulator,name=iPhone 14' + action: build CODE_SIGN_IDENTITY="" CODE_SIGNING_REQUIRED=NO diff --git a/.gitignore b/.gitignore index d983c48a5..752c7bf42 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ # .DS_Store +.yarn* # XDE # .expo/ diff --git a/IapExample/README.md b/IapExample/README.md index a3711d301..59c2930e5 100644 --- a/IapExample/README.md +++ b/IapExample/README.md @@ -86,7 +86,7 @@ You've successfully run and modified your React Native App. :partying_face: # Troubleshooting -If you can't get this to work, see the [Troubleshooting](https://reactnative.dev/docs/troubleshooting) page. +If you can't get this to work, see the [Troubleshooting](https://reactnative.dev/docs/troubleshooting) page or try disabling Flipper by setting `NO_FLIPPER=1` in your environment. (e.g. `NO_FLIPPER=1 yarn ios`) # Learn More diff --git a/IapExample/ios/Podfile b/IapExample/ios/Podfile index 9eec3506d..76b45dd35 100644 --- a/IapExample/ios/Podfile +++ b/IapExample/ios/Podfile @@ -1,3 +1,5 @@ +require 'xcodeproj' + # Resolve react_native_pods.rb with node to allow for hoisting require Pod::Executable.execute_command('node', ['-p', 'require.resolve( @@ -5,7 +7,22 @@ require Pod::Executable.execute_command('node', ['-p', {paths: [process.argv[1]]}, )', __dir__]).strip -platform :ios, min_ios_version_supported +project_path = './IapExample.xcodeproj' +project = Xcodeproj::Project.open(project_path) + +# Fetches minimum deployment target version from the project and sets it as the default +config_list = project.root_object.build_configuration_list +debug_config = config_list.build_configurations.find { |config| config.name == 'Debug' } +min_ios_version = debug_config.build_settings['IPHONEOS_DEPLOYMENT_TARGET'] +default_min_ios_version = '12.4' +if min_ios_version.nil? || min_ios_version.empty? + puts "IPHONEOS_DEPLOYMENT_TARGET not set at the project level for Debug configuration. Using default value of #{default_min_ios_version}" + min_ios_version = default_min_ios_version +else + puts "Minimum iOS version set to: #{min_ios_version}" +end +platform :ios, min_ios_version || default_min_ios_version + prepare_react_native_project! # If you are using a `react-native-flipper` your iOS build will fail when `NO_FLIPPER=1` is set. diff --git a/ios/LatestPromiseKeeper.swift b/ios/LatestPromiseKeeper.swift new file mode 100644 index 000000000..f501f5a63 --- /dev/null +++ b/ios/LatestPromiseKeeper.swift @@ -0,0 +1,47 @@ +import StoreKit + +// Only keeps latest promise, assumes older promises are not needed +// Avoids racing conditions by storing latestPromise in a thread safe var +// Cancels previous promises when new ones are added +// Should not be used when all promises are relevant (e.g. Purchases) +class LatestPromiseKeeper { + private var latestPromise: ThreadSafe<(RCTPromiseResolveBlock, RCTPromiseRejectBlock)?> = ThreadSafe(nil) + private var latestRequest: ThreadSafe = ThreadSafe(nil) + + func setLatestPromise(request: SKProductsRequest, resolve: @escaping RCTPromiseResolveBlock, reject: @escaping RCTPromiseRejectBlock) { + // Cancel the ongoing request and reject the existing promise before setting a new one + cancelOngoingRequest() + + latestRequest.atomically { $0 = request } + latestPromise.atomically { $0 = (resolve, reject) } + } + + func cancelOngoingRequest() { + latestPromise.atomically { promiseResolvers in + if let (_, reject) = promiseResolvers { + // Reject the promise with an error indicating that it was cancelled due to a new request + reject("E_CANCELED", "Previous request was cancelled due to a new request", nil) + } + } + + latestRequest.atomically { ongoingRequest in + ongoingRequest?.cancel() + ongoingRequest = nil + } + + // Clear the latestPromise after rejecting it + latestPromise.atomically { $0 = nil } + } + + func resolveIfRequestMatches(matchingRequest: SKProductsRequest, items: [[String: Any?]], operation: (RCTPromiseResolveBlock, [[String: Any?]]) -> Void) { + latestPromise.atomically { promiseResolvers in + guard let (resolve, _) = promiseResolvers else { return } + + latestRequest.atomically { ongoingRequest in + guard ongoingRequest === matchingRequest else { return } + + operation(resolve, items) + } + } + } +} diff --git a/ios/RNIapIos.swift b/ios/RNIapIos.swift index a18121f0a..6d1074a16 100644 --- a/ios/RNIapIos.swift +++ b/ios/RNIapIos.swift @@ -8,10 +8,11 @@ class RNIapIos: RCTEventEmitter, SKRequestDelegate, SKPaymentTransactionObserver private var hasListeners = false private var pendingTransactionWithAutoFinish = false private var receiptBlock: ((Data?, Error?) -> Void)? // Block to handle request the receipt async from delegate - private var validProducts: [String: SKProduct] + private var validProducts: ThreadSafe<[String: SKProduct]> private var promotedPayment: SKPayment? private var promotedProduct: SKProduct? private var productsRequest: SKProductsRequest? + private let latestPromiseKeeper = LatestPromiseKeeper() private var countPendingTransaction: Int = 0 private var hasTransactionObserver = false @@ -19,7 +20,7 @@ class RNIapIos: RCTEventEmitter, SKRequestDelegate, SKPaymentTransactionObserver promisesByKey = [String: [RNIapIosPromise]]() pendingTransactionWithAutoFinish = false myQueue = DispatchQueue(label: "reject") - validProducts = [String: SKProduct]() + validProducts = ThreadSafe<[String: SKProduct]>([:]) super.init() addTransactionObserver() } @@ -148,7 +149,7 @@ class RNIapIos: RCTEventEmitter, SKRequestDelegate, SKPaymentTransactionObserver stopObserving() rejectAllPendingPromises() receiptBlock = nil - validProducts.removeAll() + validProducts.atomically { $0.removeAll() } promotedPayment = nil promotedProduct = nil productsRequest = nil @@ -162,10 +163,12 @@ class RNIapIos: RCTEventEmitter, SKRequestDelegate, SKPaymentTransactionObserver ) { let productIdentifiers = Set(skus) productsRequest = SKProductsRequest(productIdentifiers: productIdentifiers) + if let productsRequest = productsRequest { productsRequest.delegate = self - let key: String = productsRequest.key - addPromise(forKey: key, resolve: resolve, reject: reject) + + self.latestPromiseKeeper.setLatestPromise(request: productsRequest, resolve: resolve, reject: reject) + productsRequest.start() } } @@ -189,7 +192,7 @@ class RNIapIos: RCTEventEmitter, SKRequestDelegate, SKPaymentTransactionObserver reject: @escaping RCTPromiseRejectBlock = { _, _, _ in } ) { pendingTransactionWithAutoFinish = andDangerouslyFinishTransactionAutomatically - if let product = validProducts[sku] { + if let product = validProducts.value[sku] { addPromise(forKey: product.productIdentifier, resolve: resolve, reject: reject) let payment = SKMutablePayment(product: product) @@ -254,7 +257,7 @@ class RNIapIos: RCTEventEmitter, SKRequestDelegate, SKPaymentTransactionObserver reject: @escaping RCTPromiseRejectBlock = { _, _, _ in } ) { debugMessage("clear valid products") - validProducts.removeAll() + validProducts.atomically { $0.removeAll() } resolve(nil) } @@ -348,23 +351,26 @@ class RNIapIos: RCTEventEmitter, SKRequestDelegate, SKPaymentTransactionObserver // StoreKitDelegate func productsRequest(_ request: SKProductsRequest, didReceive response: SKProductsResponse) { + // Add received products for prod in response.products { add(prod) } var items: [[String: Any?]] = [[:]] - for product in validProducts.values { + for product in validProducts.value.values { items.append(getProductObject(product)) } - resolvePromises(forKey: request.key, value: items) + self.latestPromiseKeeper.resolveIfRequestMatches(matchingRequest: request, items: items) { (resolve, items) in + resolve(items) + } } // Add to valid products from Apple server response. Allowing getProducts, getSubscriptions call several times. // Doesn't allow duplication. Replace new product. func add(_ aProd: SKProduct) { debugMessage("Add new object: \(aProd.productIdentifier)") - validProducts[aProd.productIdentifier] = aProd + validProducts.atomically { $0[aProd.productIdentifier] = aProd } } func request(_ request: SKRequest, didFailWithError error: Error) { @@ -395,17 +401,14 @@ class RNIapIos: RCTEventEmitter, SKRequestDelegate, SKPaymentTransactionObserver switch transaction.transactionState { case .purchasing: debugMessage("Purchase Started") - break case .purchased: debugMessage("Purchase Successful") purchaseProcess(transaction) - break case .restored: debugMessage("Restored") SKPaymentQueue.default().finishTransaction(transaction) - break case .deferred: debugMessage("Deferred (awaiting approval via parental controls, etc.)") @@ -464,8 +467,6 @@ class RNIapIos: RCTEventEmitter, SKRequestDelegate, SKPaymentTransactionObserver message: nsError?.localizedDescription, error: nsError) }) - - break } } } @@ -707,22 +708,18 @@ class RNIapIos: RCTEventEmitter, SKRequestDelegate, SKPaymentTransactionObserver case .freeTrial: paymendMode = "FREETRIAL" numberOfPeriods = String(discount.subscriptionPeriod.numberOfUnits) - break case .payAsYouGo: paymendMode = "PAYASYOUGO" numberOfPeriods = String(discount.numberOfPeriods) - break case .payUpFront: paymendMode = "PAYUPFRONT" numberOfPeriods = String(discount.subscriptionPeriod.numberOfUnits ) - break default: paymendMode = "" numberOfPeriods = "0" - break } switch discount.subscriptionPeriod.unit { @@ -746,15 +743,10 @@ class RNIapIos: RCTEventEmitter, SKRequestDelegate, SKPaymentTransactionObserver switch discount.type { case SKProductDiscount.Type.introductory: discountType = "INTRODUCTORY" - break - case SKProductDiscount.Type.subscription: discountType = "SUBSCRIPTION" - break - default: discountType = "" - break } let discountObj = [ diff --git a/ios/RNIapIos.xcodeproj/project.pbxproj b/ios/RNIapIos.xcodeproj/project.pbxproj index 26efff34d..5528c4a15 100644 --- a/ios/RNIapIos.xcodeproj/project.pbxproj +++ b/ios/RNIapIos.xcodeproj/project.pbxproj @@ -7,8 +7,9 @@ objects = { /* Begin PBXBuildFile section */ - 5E555C0D2413F4C50049A1A2 /* RNIapIos.m in Sources */ = {isa = PBXBuildFile; fileRef = B3E7B5891CC2AC0600A0062D /* RNIapIos.m */; }; - F4FF95D7245B92E800C19C63 /* RNIapIos.swift in Sources */ = {isa = PBXBuildFile; fileRef = F4FF95D6245B92E800C19C63 /* RNIapIos.swift */; }; + CBA2290C2B027DA500C780F8 /* ThreadSafe.swift in Sources */ = {isa = PBXBuildFile; fileRef = CBA2290A2B027DA500C780F8 /* ThreadSafe.swift */; }; + CBA2290D2B027DA500C780F8 /* LatestPromiseKeeper.swift in Sources */ = {isa = PBXBuildFile; fileRef = CBA2290B2B027DA500C780F8 /* LatestPromiseKeeper.swift */; }; + F4FF95D7245B92E800C19C63 /* RNIapIos.swift in Sources */ = {isa = PBXBuildFile; fileRef = F4FF95D6245B92E800C19C63 /* RNIapIos.swift */; }; /* End PBXBuildFile section */ /* Begin PBXCopyFilesBuildPhase section */ @@ -26,7 +27,9 @@ /* Begin PBXFileReference section */ 134814201AA4EA6300B7C361 /* libRNIapIos.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = libRNIapIos.a; sourceTree = BUILT_PRODUCTS_DIR; }; B3E7B5891CC2AC0600A0062D /* RNIapIos.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RNIapIos.m; sourceTree = ""; }; - F4FF95D5245B92E700C19C63 /* RNIapIosIap-Bridging-Header.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "RNIapIosIap-Bridging-Header.h"; sourceTree = ""; }; + CBA2290A2B027DA500C780F8 /* ThreadSafe.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ThreadSafe.swift; sourceTree = ""; }; + CBA2290B2B027DA500C780F8 /* LatestPromiseKeeper.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LatestPromiseKeeper.swift; sourceTree = ""; }; + F4FF95D5245B92E700C19C63 /* RNIapIos-Bridging-Header.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "RNIapIos-Bridging-Header.h"; sourceTree = ""; }; F4FF95D6245B92E800C19C63 /* RNIapIos.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RNIapIos.swift; sourceTree = ""; }; /* End PBXFileReference section */ @@ -52,9 +55,11 @@ 58B511D21A9E6C8500147676 = { isa = PBXGroup; children = ( + CBA2290B2B027DA500C780F8 /* LatestPromiseKeeper.swift */, + CBA2290A2B027DA500C780F8 /* ThreadSafe.swift */, F4FF95D6245B92E800C19C63 /* RNIapIos.swift */, B3E7B5891CC2AC0600A0062D /* RNIapIos.m */, - F4FF95D5245B92E700C19C63 /* RNIapIosIap-Bridging-Header.h */, + F4FF95D5245B92E700C19C63 /* RNIapIos-Bridging-Header.h */, 134814211AA4EA7D00B7C361 /* Products */, ); sourceTree = ""; @@ -116,8 +121,9 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + CBA2290C2B027DA500C780F8 /* ThreadSafe.swift in Sources */, + CBA2290D2B027DA500C780F8 /* LatestPromiseKeeper.swift in Sources */, F4FF95D7245B92E800C19C63 /* RNIapIos.swift in Sources */, - B3E7B58A1CC2AC0600A0062D /* RNIapIos.m in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -232,7 +238,7 @@ OTHER_LDFLAGS = "-ObjC"; PRODUCT_NAME = RNIapIos; SKIP_INSTALL = YES; - SWIFT_OBJC_BRIDGING_HEADER = "RNIapIosIap-Bridging-Header.h"; + SWIFT_OBJC_BRIDGING_HEADER = "RNIapIos-Bridging-Header.h"; SWIFT_OPTIMIZATION_LEVEL = "-Onone"; SWIFT_VERSION = 5.0; }; @@ -251,7 +257,7 @@ OTHER_LDFLAGS = "-ObjC"; PRODUCT_NAME = RNIapIos; SKIP_INSTALL = YES; - SWIFT_OBJC_BRIDGING_HEADER = "RNIapIosIap-Bridging-Header.h"; + SWIFT_OBJC_BRIDGING_HEADER = "RNIapIos-Bridging-Header.h"; SWIFT_VERSION = 5.0; }; name = Release; diff --git a/ios/ThreadSafe.swift b/ios/ThreadSafe.swift new file mode 100644 index 000000000..f90c0c30e --- /dev/null +++ b/ios/ThreadSafe.swift @@ -0,0 +1,18 @@ +class ThreadSafe { + private var _value: A + private let queue = DispatchQueue(label: "ThreadSafe") + + init(_ value: A) { + self._value = value + } + + var value: A { + return queue.sync { _value } + } + + func atomically(_ transform: (inout A) -> Void) { + queue.sync { + transform(&self._value) + } + } +}