status-im / nim-chronos

Chronos - An efficient library for asynchronous programming
https://status-im.github.io/nim-chronos/docs/chronos
Apache License 2.0
353 stars 51 forks source link

[RFC] Why aren't cancellations propagated by composite futures? #333

Open zah opened 1 year ago

zah commented 1 year ago

Pretty much all composite futures in Chronos do not propagate cancellations. For example, here is what the documentation is saying about allFutures:

  ## Returns a future which will complete only when all futures in ``futs``
  ## will be completed, failed or canceled.
  ##
  ## If the argument is empty, the returned future COMPLETES immediately.
  ##
  ## On cancel all the awaited futures ``futs`` WILL NOT BE cancelled.

The treatment is similar for and, or, all, race.

What's the rationale behind this design choice? My expectation is that composite futures should propagate cancellation. For example, imagine I have the following async proc:

proc startMultiHourWorkflow() {.async.} =
  let job1 = startMultiHourJob()
  let job2 = startAnotherMultiHourJob()
  await allFutures([job1, job2])
  ...

In my application, I'm likely to have the following code:

app.workflowFut = startMultiHourWorkflow()
...
app.onSomeEvent do:
  app.workflowFut.cancel()

Naturally, I expect cancelling workflowFut to immediately stop any remaining work of "job1" and "job2" that may take another several hours to complete. Instead, Chronos will let them run to completion unless I explicitly write cancellation propagation code in my startMultiHourWorkflow proc, which strikes me as the wrong default (not wanting the cancellation to be propagated should be a rare and unusual use case).