`Sendable` support by NachoSoto · Pull Request #1795 · RevenueCat/purchases-ios

NachoSoto · 2022-08-05T20:02:04Z

Main changes:

Sendable conformances to all types that are thread-safe
@unchecked Sendable for types that the compiler can't enforce, but with documentation as to why
Made classes that aren't mocked final (note that this doesn't change the API, because none of these were open to begin with)
Made some classes thread-safe that weren't (like DeviceCache)

For the non-final classes, I've actually managed to compile the SDK with all of them as final and Sendable, to make sure that making them @unchecked Sendable didn't hide any other issues.

Future improvements:

Depends on:

NachoSoto · 2022-08-05T20:05:19Z

We'll see if we keep this or not.

I think it's not a bad idea to keep this so we ensure we maintain this level of correctness prior to Swift 6.0

The type is now immutable, so the compiler ensures it's thread-safe. For CSDK-379. Extracted #1795 to make it easier to review this fix in isolation.

For [CSDK-379]. Extracted #1795 to make it easier to review this fix in isolation. Also made `ASN1ObjectIdentifierBuilder` an `enum` since we don't need to create instances of it to use it.

NachoSoto · 2022-08-13T17:43:31Z

This is almost ready, but it will be a lot easier to review once all those dependent PRs are merged.

The type is now immutable, so the compiler ensures it's thread-safe. For CSDK-379. Extracted #1795 to make it easier to review this fix in isolation.

The type is now immutable, so the compiler ensures it's thread-safe. For [CSDK-379]. Extracted from #1795 to make it easier to review this in isolation.

The type is now immutable, so the compiler ensures it's thread-safe. For [CSDK-379]. Extracted from #1795 to make it easier to review this in isolation. [CSDK-379]: https://revenuecats.atlassian.net/browse/CSDK-379?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

For [CSDK-379]. Extracted from #1795 to make it easier to review this in isolation. Also made `ASN1ObjectIdentifierBuilder` an `enum` since we don't need to create instances of it to use it. [CSDK-379]: https://revenuecats.atlassian.net/browse/CSDK-379?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

The type is now immutable, so the compiler ensures it's thread-safe. For [CSDK-379]. Extracted from #1795 to make it easier to review this in isolation. [CSDK-379]: https://revenuecats.atlassian.net/browse/CSDK-379?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

NachoSoto · 2022-08-22T23:04:44Z

This is ready to review @RevenueCat/core-sdk

NachoSoto · 2022-08-29T14:33:54Z

Bump @RevenueCat/core-sdk

taquitos

😮‍💨
Nice work.

NachoSoto · 2022-08-31T19:42:14Z

Merging this unless somebody else has any concerns or wants more time to look at it @RevenueCat/core-sdk

aboedo · 2022-08-31T19:56:16Z

+extension OperationDispatcher {
+
+    static func dispatchOnMainActor(_ block: @MainActor @escaping @Sendable () -> Void) {
+        if #available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) {


this is cool

aboedo · 2022-08-31T19:57:15Z

+            }
        } else {
-            workerQueue.async(execute: block)
+            DispatchQueue.main.async(execute: block)


should this be self.mainQueue?

Oh yeah good call.

Oh I know why I did this: there is some magic that allows passing a @MainActor block to only DispatchQueue.main, but not if we inject any random queue:

ohhh right, that makes sense. Could you add a comment for the future?

aboedo · 2022-08-31T19:58:18Z

    static let `default`: OperationDispatcher = .init()

-    func dispatchOnMainThread(_ block: @escaping () -> Void) {
+    func dispatchOnMainThread(_ block: @escaping @Sendable () -> Void) {


Should this be made private now? Is there any scenario where we should choose to go with main thread and not main actor even when Swift Concurrency is available?

Eventually we can get rid of this, but that seems like a bigger change. For existing code that does not use the new Swift "structured" concurrency, we might not want to change the semantics if we don't need to.

There's still a few places using dispatchOnMainThread that we might not want to change until we use @MainActor (or other custom actors) in more places.

Let me know what you think, we can migrate more of these to dispatchOnMainActor in a following PR.

@NachoSoto

**This is an automatic release.** ### Bugfixes * `watchOS`: fixed crash when ran on single-target apps with Xcode 14 and before `watchOS 9.0` (#1895) via NachoSoto (@NachoSoto) * `CustomerInfoManager`/`OfferingsManager`: improved display of underlying errors (#1888) via NachoSoto (@NachoSoto) * `Offering`: improved confusing log for `PackageType.custom` (#1884) via NachoSoto (@NachoSoto) * `PurchasesOrchestrator`: don't log warning if `allowSharingAppStoreAccount` setting was never explicitly set (#1885) via NachoSoto (@NachoSoto) * Introduced type-safe `PurchasesError` and fixed some incorrect returned error types (#1879) via NachoSoto (@NachoSoto) * `CustomerInfoManager`: fixed thread-unsafe implementation (#1878) via NachoSoto (@NachoSoto) ### New Features * Disable SK1's `StoreKitWrapper` if SK2 is enabled and available (#1882) via NachoSoto (@NachoSoto) * `Sendable` support (#1795) via NachoSoto (@NachoSoto) ### Other Changes * Renamed `StoreKitWrapper` to `StoreKit1Wrapper` (#1886) via NachoSoto (@NachoSoto) * Enabled `DEAD_CODE_STRIPPING` (#1887) via NachoSoto (@NachoSoto) * `HTTPClient`: added `X-Client-Bundle-ID` and logged on SDK initialization (#1883) via NachoSoto (@NachoSoto) * add link to SDK reference (#1872) via Andy Boedo (@aboedo) * Added `StoreKit2Setting.shouldOnlyUseStoreKit2` (#1881) via NachoSoto (@NachoSoto) * Introduced `TestLogHandler` to simplify how we test logged messages (#1858) via NachoSoto (@NachoSoto) * `Integration Tests`: added test for purchasing `StoreProduct` instead of `Package` (#1875) via NachoSoto (@NachoSoto) * `ErrorUtils`: added test to verify that returned errors can be converted to `ErrorCode` (#1871) via NachoSoto (@NachoSoto)

NachoSoto added the pr:feat A new feature label Aug 5, 2022

NachoSoto commented Aug 5, 2022

View reviewed changes

NachoSoto force-pushed the sendable branch 3 times, most recently from 0044e54 to 9bf8b8e Compare August 9, 2022 21:53

RevenueCat deleted a comment from RevenueCat-Danger-Bot Aug 9, 2022

NachoSoto force-pushed the sendable branch 2 times, most recently from abdf7a2 to 2aa6468 Compare August 9, 2022 23:09

NachoSoto requested a review from a team August 9, 2022 23:15

NachoSoto force-pushed the sendable branch 9 times, most recently from b5c1b42 to faed2ac Compare August 13, 2022 16:42

NachoSoto mentioned this pull request Aug 13, 2022

DeviceCache: Sendable conformance and fixed thread-safety #1823

Merged

NachoSoto added a commit that referenced this pull request Aug 13, 2022

CustomerInfo: Sendable conformance

3b91640

The type is now immutable, so the compiler ensures it's thread-safe. For CSDK-379. Extracted #1795 to make it easier to review this fix in isolation.

NachoSoto mentioned this pull request Aug 13, 2022

CustomerInfo: Sendable conformance #1824

Merged

NachoSoto mentioned this pull request Aug 13, 2022

ReceiptParser: Sendable conformance #1825

Merged

NachoSoto force-pushed the sendable branch 2 times, most recently from 7931d7e to 0e2538d Compare August 13, 2022 17:31

NachoSoto mentioned this pull request Aug 13, 2022

Offering/StoreProductType: Sendable conformance #1826

Merged

NachoSoto force-pushed the sendable branch 2 times, most recently from 9bd795e to cc09992 Compare August 16, 2022 16:24

NachoSoto added a commit that referenced this pull request Aug 16, 2022

CustomerInfo: Sendable conformance

7316c91

The type is now immutable, so the compiler ensures it's thread-safe. For CSDK-379. Extracted #1795 to make it easier to review this fix in isolation.

NachoSoto added a commit that referenced this pull request Aug 16, 2022

Offering: Sendable conformance

4bf7df9

The type is now immutable, so the compiler ensures it's thread-safe. For [CSDK-379]. Extracted from #1795 to make it easier to review this in isolation.

NachoSoto force-pushed the sendable branch 3 times, most recently from 51387b7 to 5645f05 Compare August 16, 2022 17:33

NachoSoto mentioned this pull request Aug 16, 2022

Improve Swift Concurrency support #1041

Closed

NachoSoto force-pushed the sendable branch from 45f7652 to e5014a6 Compare August 22, 2022 23:02

NachoSoto force-pushed the sendable branch from e5014a6 to fabb158 Compare August 29, 2022 23:54

NachoSoto added 6 commits August 31, 2022 10:54

[WIP] Sendable support

3dbf5eb

TestCase: @MainActor

18bfc99

Fixed integration tests

ff0b7cb

XCTAsyncUnwrap: @MainActor instead of @Sendable

be3b3c1

Fixed a few warnings in Xcode beta 6

b03899a

Fixed pre-iOS 16 build

347d2e6

NachoSoto force-pushed the sendable branch from 71a5312 to 347d2e6 Compare August 31, 2022 18:00

taquitos approved these changes Aug 31, 2022

View reviewed changes

Comment thread Sources/Purchasing/Purchases/Purchases.swift

Comment thread Sources/Purchasing/Purchases/PurchasesOrchestrator.swift

Comment thread Sources/Purchasing/TrialOrIntroPriceEligibilityChecker.swift

aboedo approved these changes Aug 31, 2022

View reviewed changes

aboedo reviewed Aug 31, 2022

View reviewed changes

NachoSoto merged commit 0bff3f6 into main Aug 31, 2022

NachoSoto deleted the sendable branch August 31, 2022 20:30

revenuecat-ops mentioned this pull request Sep 7, 2022

[AUTOMATIC] Release/4.12.0 #1890

Merged

Conversation

NachoSoto commented Aug 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Main changes:

Future improvements:

Depends on:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NachoSoto commented Aug 13, 2022

Uh oh!

NachoSoto commented Aug 22, 2022

Uh oh!

NachoSoto commented Aug 29, 2022

Uh oh!

taquitos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NachoSoto commented Aug 31, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NachoSoto Aug 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NachoSoto commented Aug 5, 2022 •

edited

Loading

NachoSoto Aug 31, 2022 •

edited

Loading