PurchasesOrchestrator: replaced manual Lock with Atomic#1664
Conversation
86c2abb to
2ed8c36
Compare
See #983. This is less error-prone as it's impossible to have deadlocks, or to access the data without locking (like we were doing in some cases). Also, this approach has 2 separate locks for each property, so unrelated accesses won't lock each other now.
2ed8c36 to
2bd983c
Compare
taquitos
left a comment
There was a problem hiding this comment.
I THINK there's a change here you should make, but let me know what you think.
| delegate.readyForPromotedProduct(storeProduct) { completion in | ||
| self.purchaseCompleteCallbacksByProductID[product.productIdentifier] = completion | ||
| self.purchaseCompleteCallbacksByProductID.modify { $0[product.productIdentifier] = completion } | ||
| storeKitWrapper.add(payment) |
There was a problem hiding this comment.
I kinda think it makes sense to add this in to the .modify block since that guarantees the same behavior we had with the lock.
There was a problem hiding this comment.
The thing is this is an @escaping block, so the outside lock was likely getting unlocked before this was even modified. Does that make sense?
So the access to purchaseCompleteCallbacksByProductID wasn't protected. I do see that maybe the intent is that the whole thing was, and therefore maybe storeKitWrapper.add(payment) could be inside now. But considering that was missed, and therefore this is the same behavior, maybe this makes more sense?
_This version supports Xcode 14 beta 1_ * `PurchasesOrchestrator.handleDeferredTransaction`: check `NSError.domain` too (#1665) via NachoSoto (@NachoSoto) * `PurchasesOrchestrator`: replaced manual `Lock` with `Atomic` (#1664) via NachoSoto (@NachoSoto) * `CodableStrings.decoding_error`: added underlying error information (#1668) via NachoSoto (@NachoSoto) * Fixed Xcode 14 compilation: avoid `@available` properties (#1661) via NachoSoto (@NachoSoto)
See #983. This is less error-prone as it's impossible to have deadlocks, or to access the data without locking (like we were doing in some cases).
Also, this approach has 2 separate locks for each property, so unrelated accesses won't lock each other now.