AsyncCompatibilityKit icon indicating copy to clipboard operation
AsyncCompatibilityKit copied to clipboard

Actor

Open robertmryan opened this issue 3 years ago • 5 comments

  • The use of the onCancel closure is hiding the fact that the completion handler is @Sendable, whereas onCancel is not.
  • If you define onCancel to be @Sendable, it reintroduces the compiler warning.
  • I suspect that the closure pattern was used to eliminate the compiler warning, but I believe you are simply eliminating the warning, but not fixing the problem that the compiler was warning us about.
  • Instead, use an actor to ensure thread-safe interaction with the URLSessionTask.

robertmryan avatar Jan 03 '22 19:01 robertmryan

If you enable additional concurrency warnings in Xcode, you can see that access to onCancel in the original implementation causes a warning:

Cannot use let 'onCancel' with a non-sendable type '() -> Void?' from concurrently-executed code

Warnings can be enabled with these flags:

"OTHER_SWIFT_FLAGS": "-Xfrontend -warn-concurrency -Xfrontend -enable-actor-data-race-checks"

Looking forward for John's opinion on this

voidless avatar Jan 31 '22 14:01 voidless

In Xcode 13.3 (Beta 2) this still triggers warnings:

Capture of 'request' with non-sendable type 'URLRequest' in a `@Sendable` closure
Capture of 'self' with non-sendable type 'URLSession' in a `@Sendable` closure
Non-sendable type 'URLSessionTask' passed in implicitly asynchronous call to actor-isolated instance method 'start' cannot cross actor boundary
Capture of 'continuation' with non-sendable type 'CheckedContinuation<(Data, URLResponse), Error>' in a `@Sendable` closure

futuretap avatar Feb 17 '22 20:02 futuretap

@futuretap - I don’t think that the warnings triggered by this second beta are easily resolved because it is not recognizing clearly sendable objects as such (e.g., an immutable URLRequest constant is clearly capturable, but beta 2 is not allowing this). I’d suggest we keep an eye out for future betas. Hard to fix right now.

robertmryan avatar Feb 18 '22 15:02 robertmryan

BTW, @futuretap, the main branch generates the same sort of compiler warning. It is not an issue limited to this PR and Xcode 13.3 Beta 2.

robertmryan avatar Feb 18 '22 15:02 robertmryan

If we are absolutely sure that URLRequest and URLSession should be Sendable, we can silence that warning like this:

extension URLRequest: @unchecked Sendable {}
extension URLSession: @unchecked Sendable {}

voidless avatar Feb 21 '22 16:02 voidless