Mark Sendable for NetworkContext's external async calls#36
Conversation
tfpauly
requested review from
agnosticdev,
ekinnear,
kkuk24 and
rpaulo
as code owners
tfpauly
force-pushed
the
tfp/sendable-context-async
branch
from
5ed13e2 to
b16423c
Compare
|
|
||
| @available(Network 0.1.0, *) | ||
| final class EndpointFlow: CustomDebugStringConvertible { | ||
| final class EndpointFlow: CustomDebugStringConvertible, @unchecked Sendable { |
There was a problem hiding this comment.
In NIO we have a rule that any use of @unchecked Sendable must include the rationale for why they are @unchecked. It's been incredibly valuable. Our long term goal is to remove all uses of @unchecked. You may find it helpful to adopt a similar pattern here.
Using @unchcked is often a footgun: it's easy to add new properties which aren't Sendable but might, for example, require being on the context to access safely. In NIO we usually isolate any state like that to its own struct within the type and only mark that as @unchecked but include runtime checks (for debug builds) to make sure we aren't violating those contracts. It was actually one such check that led us to this PR.
| ) { | ||
| let endpointFlow = self.endpointFlow | ||
| nonisolated(unsafe) let unsafeBuffer = buffer | ||
| nonisolated(unsafe) let unsafeOwner = owner |
There was a problem hiding this comment.
Are we able to build the WriteRequest and then pass it into the block to avoid the nonisolated(unsafe) ?
3 participants
Adding Sendable marking to the async block for NetworkContext that can be used from off-context. The async call within the stack to schedule a block on the context itself validates that it is on the right context already, and is now called "isolatedAsync" within the NetworkContext.
This also requires a lot of changes to mark unsafe sendability in the tests that use async heavily along with expectations to do step-by-step testing.