[concurrency] What obligations do executors have around task priority?

Using Swift
1

This is a question, but maybe also a bug report.

At my employer, we use a combination of swift_task_enqueueGlobal_hook and Task priorities to manage the order that jobs are executed in. A coworker was recently examining a test harness that uses this strategy, and decided to see what would happen if he removed our task_enqueue hook (which moves everything to MainActor). He was surprised to discover that the code immediately crashed in Task.init.

We later narrowed the problem down the following reproducer:

await withTaskGroup { group in
    group.addTask {
        Task(
            priority: .init(rawValue: 28)
        ) {} // crashes in `dispatch_async_swift_job
    }
}

In the Swift 5 language mode, this code also causes the same crash:

Task(
    priority: .init(rawValue: 28)
) {}

But it doesn't happen in Swift 6. I'm not clear on why.

A "fix" for these is to only use "official" built in values like .low, .high, or .userInitiated. Another "fix" is to mark the Task as @MainActor, implying that how this function behaves is down to the implementation of the executor that's in use.

We went looking for an explanation and were surprised to discover that it uses some part of MacOS/iOS that isn't part of the Swift project. Not sure if this function is open source or not.

Another oddity: there seems to be a hard limit on how high the job priorities can go: a value of 240 crashes with the error **invalid job priority 0xf0** thrown.

So my question is: what's the deal? Why does this crash? Is it legitimate for actors to choose a range of values they accept for priority and crash if they receive the wrong value? Is the current behavior expected or intended?

1 Like
2

Interestingly, you can also cause the same error with a plain detached task:

@main
enum App {
  static func main() async throws {
    Task.detached(priority: .init(rawValue: 34)) {} // Crash

    Task(priority: .init(rawValue: 255)) {} // Okay
  }
}

Although, when running the code in Godbolt, the value must be 34 or higher.

Edit: Haven’t looked into it further yet, but this should be the function responsible for the crash: swift/stdlib/public/Concurrency/DispatchGlobalExecutor.cpp at 61ed6bd8e0be3482a2aa480b988a2c4f0b28aeac · swiftlang/swift · GitHub

2 Likes
3

Hmm, the fact that we allow arbitrary priority values is not entirely accidental, but also has not really been tested much in reality.

The problem you're facing is that how should an executor who gets such unknown value treat it; we've not really defined this anywhere, so I expect the default dispatch executors to just be "surprised" and blow up like this if you will...

We could just say they "round down" or something like this...?

Could you talk a bit more about the use case here? Why would we want to keep allowing arbitrary values like this, is it to just "try out if you can get back to default executors and remove your global hook"?

Though the same issue I think exists once we have more custom executors, including the global ones which @al45tair worked on hm.

1 Like
4

Had another look at it, the max allowed value (33) maps directly to the highest "official" value; userInteractive. From the crash condition it looks like each allowed priority has its own queue. I agree with Konrad, perhaps, instead of crashing we could just downgrade the value to the highest allowed one? @al45tair correct me if I'm wrong.

5

The new Swift-based executors tend to use the PriorityQueue implementation I put into the Concurrency library and thus can cope with arbitrary priorities, but the Dispatch executor does have a fixed set of priorities it allows and will fatal error if you pass something outside of that range.

The Win32ThreadPoolExecutor implementation in swift-platform-executors is probably the most interesting case, in that that one does map the specified priority to one of the three priorities supported by the Win32 ThreadPool (specifically, TaskPriority.low and below map to low priority, TaskPriority.high and above map to high priority, and everything else is normal priority). Arguably the Dispatch executor should behave in a similar manner.

3 Likes
6

Thanks for the explanation! I think unifying behavior between the different implementations would be reasonable.

1 Like
7

Okay, so I realize now that this is actually documented on TaskPriority, and I should have read that. To clarify things; my assumption was that Task Priority was essentially just a typealias for a number, and that all the existing values with a name (.low, .high, etc) were effectively just suggestions to users, similar to how NSLayoutConstraint.priority works. Which, to be fair, is apparently accurate for some executors.

I realize this is impossible, but a better API would be for Actors to define an associatedType for priority, and then it would always be clear what values were acceptable in any given context. It seems like it would be basically impossible to do this given that the isolation isn't in the type system anymore in cases like

func makeTask(isolation: isolated (any Actor)? = #isolation {
    Task(priority: .low) { _ = isolation}
}

So we're stuck with what is essentially a union of all possible valid priorities, and it's on the author of a program to make sure they're not calling code that can be isolated to any actor, but has a priority that is not accepted by that executor.

To be fair, as an iOS developer I don't know any programs that would be likely to run into this issue; I think even my testing harness is a special case, and to be honest it's unclear if we needed these custom priorities in the first place.

Could you talk a bit more about the use case here? Why would we want to keep allowing arbitrary values like this, is it to just "try out if you can get back to default executors and remove your global hook"?

Given that the values were accepted by the main actor, there isn't a use case as such, this is mainly curiosity on my part. My coworker was experimenting to see how much of the test harness code he could remove and still have passing tests, and we ran into this. Which surprised us because generally speaking when Swift APIs crash there's a very obvious "you're holding it wrong" kind of moment, but in this case the error messages were opaque and the rationale for crashing wasn't obvious.

1 Like
8

I remember asking about this while the initial proposals on Swift Concurrency were discussed, but this turns out to immediately be unviable.

Not even because it's impossible to retroactively redesign this now, but because a program can have arbitrarily many executors (some being private to some very deeply hidden dependency, so one won't be able to construct that associated type to begin with), and any task can visit arbitrarily many of those—possibly all—so there must be one single, totally ordered type that all executors agree upon.

1 Like
9

Yeah so priorities cannot be tied to type system because you never know at compile time where you might be running in e.g. a synchronous function, since it can be called on various executors. And even (default) actor executors may be overriden by task executors. Priority remains a dynamic runtime thing...

What I think may be actionable though is try to offer better runtime failure or tolerance in the default executors -- that could use an issue on the issue tracker if you'd like to file something about it.

2 Likes
10

And even (default) actor executors may be overriden by task executors

Ah, I forgot about that API, on top of the reason I had in my post.

What I think may be actionable though is try to offer better runtime failure or tolerance in the default executors

Filed a bug here.

1 Like