swift-async-algorithms icon indicating copy to clipboard operation
swift-async-algorithms copied to clipboard

`Task.select` loser-tasks cancellation

Open inamiy opened this issue 3 years ago • 5 comments

Current TaskSelect.swift impl does not seem to support loser-tasks cancellation.

For example:

final class TestTaskSelect: XCTestCase {
  func test_first() async throws {
    let firstValue = try await Task.select(Task { // Winner task
      return 1
    }, Task { // Loser task
      try await Task.sleep(until: .now + .seconds(2), clock: .continuous)

      fatalError("Should not reach here") // ADDED

      return 2
    }).value

    try await Task.sleep(/* wait for both completion & cancellation */) // ADDED

    XCTAssertEqual(firstValue, 1)
  }
}

(Modified from original test code) When first task is completed, second task should be automatically cancelled, so fatalError should not be reached.

Is this an intended impl or considered a bug?

inamiy avatar Mar 26 '22 08:03 inamiy

P.S. For reference, Rust's tokio also has select! macro that has similar behavior as Task.select, which will cancel other works when first one completes.

  • https://rust-lang.github.io/async-book/06_multiple_futures/03_select.html

Also, here's Swift async-version of task racing (which can be replaced with Task.select if loser-task cancellation is supported.

  • https://github.com/inamiy/FunAsync/blob/main/Sources/FunAsync/AsyncFirst.swift

inamiy avatar Mar 26 '22 08:03 inamiy

The behavior you are describing is more like TaskGroup.

phausler avatar Mar 27 '22 15:03 phausler

@phausler

The behavior you are describing is more like TaskGroup.

TaskGroup only cancels other tasks when error is received, not when successful completion. (You might be mentioning about the same approach as AsyncFirst.swift)

As I mentioned above, auto-cancellation of loser-tasks is more commonly supported in many frameworks just because developers normally don't care about the loser's value.

On the other hand, when I looked at the internal code usage e.g. AsyncCombineLatest2Sequence, I also noticed that this proposed auto-cancellation will cause the breaking of internal behavior.

So, for internal use case, current Task.select looks good as is, and for end-user's case, supporting auto-cancellation will be even better. (I'm not sure about current Task.select with requirement of manual-cancellation will be a nice API for end-users)

inamiy avatar Mar 28 '22 01:03 inamiy

yea that AsyncFirst is precisely the usage I was thinking about for task groups. Task.select is a shape that TaskGroup flat-out can't do precisely because of how the cancellation works. I have a feeling it falls into the category of "slightly more advanced API" where the progressive disclosure goes from: async functions -> async let -> Task -> TaskGroup -> Task.select.

The name select was chosen because it kinda has the same behavior as the C api select. Looser sockets in the FD_SET don't get closed; they stay open for reading in the next call to select.

I do feel that TaskGroup can be a bit harder to use than perhaps it ought to be. Perhaps we can find a shape that can make both of them better?

phausler avatar Mar 28 '22 04:03 phausler

The name select was chosen because it kinda has the same behavior as the C api select. Looser sockets in the FD_SET don't get closed; they stay open for reading in the next call to select.

That's good to know! So, the naming of Task.select also makes sense (cf. discussion in https://github.com/apple/swift-async-algorithms/issues/107).

To summarize above:

  • Task.select has its own minimal purpose (without cancellation) that is needed internally, and possibly useful for end-users
    • I don't come up with good use case though (maybe when end-user also makes another new Rx operator?)

To extend this "automatic loser-cancellation" topic, I think there are following approaches:

  1. Define new API, e.g. Task.race()
    • race may be good naming to terminate loser tasks
  2. Add extra argument, e.g. Task.select(racing: true, ...)
  3. No extension (let developers cancel loser tasks manually)

I prefer either 1 or 2 to be achieved in this library, and would like to hear more thoughts on this.

inamiy avatar Mar 29 '22 01:03 inamiy

We removed Task.select since it encouraged the usage of unstructured concurrency; hence, closing this issue.

FranzBusch avatar Jun 15 '23 09:06 FranzBusch