Remove race condition in Scheduler.effect.delayCancellable

[SystemFw]

Fixes #1077

Scheduler.effect.timedCancellable schedules an action that completes a Promise with Some(a) after a timer completes, and returns the result of reading the Promise along with an action that cancel s the timer and completes the Promise with None.
This means that if cancel is called after the timer and the action have completed, it will try to complete the Promise with None after the action has already completed it with Some(a), and therefore the cancel action will fail with a PromiseAlreadyCompletedException.

If you look at timedGet (or timedDecrementBy in Semaphore, which has the same problem), we can see where the problem is:

cancellableGet.flatMap {
      case (g, cancelGet) =>
        scheduler.effect.delayCancellable(F.unit, timeout).flatMap {
          case (timer, cancelTimer) =>
            fs2.async
              .race(g, timer)
              .flatMap(_.fold(a => cancelTimer.as(Some(a)), _ => cancelGet.as(None)))
        }
    }

If g wins the race, then cancelTimer.as(Some(a)) is executed, but if in the meantime timer has completed (so it lost the race, but it completed before cancelTimer executes), then cancelTimer.as(Some(a)) will be a failed F as explained above, and therefore it will short circuit over the as(Some(a)) and result in the whole timedGet failing with a PromiseAlreadyCompleted exception.

The fix is to make sure that cancelTimer is a no op if called after the timer has already completed, instead of having it fail.

[SystemFw]

There's a failure in a doctest for merge which, while involving a Scheduler, seems to be unrelated (I don't think anyhing in the call chain there is calling delayCancellable)

[mpilquist]

Good find!

[ChristopherDavenport]

Amazing job closing this one out so quickly. Thank you very much for taking care of this!

[SystemFw]

My pleasure :)