delay(0.seconds) does not check for cancellation

[dziemborowicz]

The delay function claims to provide a prompt cancellation guarantee, but, if the duration passed to delay is 0, the function simply returns without doing anything and, in particular, without checking for cancellation.

Should this function be updated to check for cancellation even in the case where it does not suspend so that it guarantees that it never returns successfully if the job was cancelled before it was called?

Or, if not, should the documentation here be updated to note that delay does not check for cancellation if the duration is not strictly positive?

[qwwdfsad]

delay behaves exactly according to its documentation:

There is a prompt cancellation guarantee If the job was cancelled while this function was suspended, it will not resume successfully

This is the default behaviour of most of our primitives -- cancellation is "checked" only during actual suspension for the sake of performance.

Could you please elaborate on whether you spotted it accidentally or if there was an actual production-driven use-case for that?

[dziemborowicz]

You're right that the documentation says that it only checks for cancellation if it actually suspends. And it makes some sense that it doesn't suspend if the delay is 0, but this second fact isn't documented explicitly.

I shot myself in the foot with this because I hadn't considered the possibility that it wouldn't check for cancellation in the 0 case. I had some utility that would execute something repeatedly while in a certain state. In simplified terms --

fun every(duration: Duration, action: () -> Unit) {
  stateScope.launch {
    while (true) {
      delay(duration)
      action()
    }
  }
}

The idea was that when the state was no longer current, stateScope would be cancelled elsewhere and the launched coroutine would stop. If the caller passed in 0 for the duration, however, the coroutine would run forever since nothing checked for cancellation. (Trivially fixable with an isActive check of course. It was caught by unit tests.)

This has similar tones to the special cases that used to be a problem with JavaScript's setTimeout(..., 0). Documenting the special 0 case here (or removing the special case?) might save someone else's foot.