paypal / paypal-ios

One merchant integration point for all of PayPal's services
Apache License 2.0
59 stars 27 forks source link

Fix Sendable Warnings with Strict Concurrency Checking #215

Closed KunJeongPark closed 3 months ago

KunJeongPark commented 11 months ago

Reason for changes

This PR addresses compiler warnings related to strict concurrency checking option set to complete. This PR is dependent on https://github.com/paypal/paypal-ios/pull/214 which addresses reason for moving toward more concurrency safety, adhering to stricter concurrency checking in Xcode.

Sendable protocol in Swift is part of the concurrency model introduced to ensure thread safety. It is a way of marking types that are safe to transfer across concurrent execution contexts. Adopting Sendable protocol is a guarantee to the compiler that instances of this type can be safely shared between different threads without risk of data race.

A type is Sendable if

  1. It is a value type (like a struct or enum) that only contains Sendable types.
  2. It's a class where all properties are Sendable, and the class itself ensures proper synchronization for its mutable state.

By default, simple value types like "Int", "String", and "Array" (when they contain Sendable types) conform to Sendable. For custom types, developers need to ensure thread safety and then mark their types as Sendable to inform both the compiler and other developers that these types can be safely used in concurrent code.

Sendable protocol does not by itself enforce any runtime checks or provide any thread-safety features. Instead, it's a static type system feature that helps the compiler verify and enforce the safety of concurrent code at compile time.

This PR's fixes addressed "non-sendable type cannot cross actor boundary" warning and "capture of non-sendable type" in Task blocks with types that don't conform to Sendable

"non-sendable type cannot cross actor boundary" Actors in Swift are a feature that provides a way to protect shared mutable state by serializing access to the state they encapsulate. When the warning says "non-sendable type cannot cross actor boundary", it's indicating that you're trying to use a type that hasn't been marked as safe for concurrent use (Sendable) in an actor, which is designed to work with Sendable types only. This could potentially lead to unsafe concurrent access to the actor's state.

"capture of non-sendable type" Task blocks in Swift are constructs that allow you to perform work asynchronously. When you use a task block, you're creating a new asynchronous context. This means that the code within the task block can run concurrently with other code. Because of this concurrency, we need to make sure that any data we access within a task block is safe to use from multiple threads or execution contexts. This is the reason for capture warnings for non-Sendable types within the Task block.

Unchecked Sendable The @unchecked Sendable conformance is a way to suppess the compiler's warnings about a type not being Sendable. Using this is a promise that usage of the type is safe, even though not all the enclosed types conform to Sendable

I'm open to discussions about how strict we want to be about these warnings. I think it's a balance between amount of code change and blanket hand waving, assuring that everything is handled correctly to avoid data race.

Summary of changes

Unfixed warnings:

Checklist

~- [ ] Added a changelog entry~

Authors

List GitHub usernames for everyone who contributed to this pull request.

scannillo commented 8 months ago

Is this PR still active? Or can we close it

jaxdesmarais commented 3 months ago

Closing due to inactivity, when we are ready to prioritize this work we can reopen this PR or start a new PR using this as a reference