Skip to content

PriceFormatterProvider: Sendable conformance and fixed thread-safety#1806

Merged
NachoSoto merged 3 commits into
number-formatter-sendablefrom
number-formatter-sendable-2
Aug 13, 2022
Merged

PriceFormatterProvider: Sendable conformance and fixed thread-safety#1806
NachoSoto merged 3 commits into
number-formatter-sendablefrom
number-formatter-sendable-2

Conversation

@NachoSoto

@NachoSoto NachoSoto commented Aug 9, 2022

Copy link
Copy Markdown
Contributor

For CSDK-379.

This class was used all over the place but it wasn't thread-safe. The Swift compiler enforces this now that it conforms to Sendable (note it's not unchecked!).
Because Atomic guarantees correctness, this thread-safety can be enforced now by the compiler.

@NachoSoto NachoSoto added the pr:fix A bug fix label Aug 9, 2022
@NachoSoto NachoSoto requested a review from a team August 9, 2022 17:05
@NachoSoto NachoSoto force-pushed the number-formatter-sendable branch from c6208d6 to e2048c7 Compare August 9, 2022 17:08
This class was used all over the place but it wasn't thread-safe. The Swift compiler enforces this now that it conforms to Sendable (note it's not unchecked!).
Because `Atomic` guarantees correctness, this thread-safety can be enforced now by the compiler.
@NachoSoto NachoSoto force-pushed the number-formatter-sendable-2 branch from d078ed5 to d0d818d Compare August 9, 2022 17:08
Comment thread Sources/Misc/Lock.swift
@NachoSoto NachoSoto changed the title PriceFormatterProvider: Sendable conformance PriceFormatterProvider: Sendable conformance and fixed thread-safety Aug 9, 2022
@NachoSoto NachoSoto mentioned this pull request Aug 9, 2022
Comment thread Sources/Misc/Lock.swift
#if swift(>=5.7)
extension Lock: Sendable {}
#else
// `NSRecursiveLock` isn't `Sendable` until iOS 16.0 / Swift 5.7

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@NachoSoto NachoSoto merged this pull request into number-formatter-sendable Aug 13, 2022
@NachoSoto NachoSoto deleted the number-formatter-sendable-2 branch August 13, 2022 15:19
@NachoSoto NachoSoto restored the number-formatter-sendable-2 branch August 13, 2022 15:20
NachoSoto added a commit that referenced this pull request Aug 31, 2022
Fixes [CSDK-379] and #1041 (comment)

[CSDK-379]: https://revenuecats.atlassian.net/browse/CSDK-379?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

### 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` `class`es, 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:
- https://twitter.com/nachosoto/status/1557139992056500224?s=21&t=arWdvEzTIFANBvwqQ0vPiA
- https://twitter.com/nachosoto/status/1557141944777592832?s=21&t=arWdvEzTIFANBvwqQ0vPiA
- https://twitter.com/nachosoto/status/1557143374922059776?s=21&t=arWdvEzTIFANBvwqQ0vPiA

### Depends on:
- #1794
- #1804
- #1806
- #1807
- #1808
- #1813
- #1822
- #1823
- #1824
- #1825
- #1826
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants