[Concurrency] Correct isolation handling with default act and task exec #82942
Conversation
ktoso
force-pushed
the
wip-task-executor-bug-isolation
branch
2 times, most recently
from
599e5f3 to
e475ee6
Compare
| @@ -461,6 +461,14 @@ public protocol TaskExecutor: Executor { | |||
| // work-scheduling operation. | |||
| @_nonoverride | |||
| func enqueue(_ job: consuming ExecutorJob) | |||
|
|
|||
| #if !$Embedded | |||
| // Must implement this requirement in order | |||
There was a problem hiding this comment.
Unfinished comment? ("in order…" to what?)
| // Must implement this requirement in order | ||
| @_nonoverride | ||
| @available(StdlibDeploymentTarget 6.2, *) | ||
| func enqueue(_ job: consuming ExecutorJob, isolatedTo serialExecutor: UnownedSerialExecutor/*some SerialExecutor*/) |
There was a problem hiding this comment.
Should there be an implementation of this in DispatchExecutor.swift? It's not obvious to me what the isolatedTo: argument actually does here, or how an executor implementation would use it. Maybe some doc comments might help?
There was a problem hiding this comment.
So my current attempt was such that you'd have to forward it into
job.run(isolatedTo: serialExecutor, serialExecutor: self.asUnownedSerialExecutor() when you get the enqueue call on the task excecutor.
I'm going to give this another look if we maybe could enqueue on the SerialExecutor when we are a DefaultActor and there notice the task executor after we aquired the default actor lock 🤔
In preparation for removal. This spelling of Task.immediate was before SE-review and was NOT accepted and therefore we should try to remove it entirely.
We did not properly account for default actor's isolation in our task executor forwarding and... this is quite unfortunate. This PR also includes a Task.immediate API addition which I'll rip out from this PR and send in independently as that's low risk and just missing API, and the bug we may have to run through an evolution amendment
ktoso
force-pushed
the
wip-task-executor-bug-isolation
branch
from
e475ee6 to
6c97c5a
Compare
We did not properly account for default actor's isolation in our task executor forwarding and... this is quite unfortunate.
TODO: rebase it on top of #82913 which does the Task.immediate changes