Release loser eagerly in Resource.race
#3226
Conversation
| cancelLoser(f).start.flatMap { f => | ||
| fa.map { | ||
| case (a, fin) => | ||
| ( | ||
| Either.left[A, B](a), | ||
| fin(_: ExitCase).guarantee(f.join.flatMap(_.embedNever))) | ||
| } | ||
| } |
There was a problem hiding this comment.
Unlike racing IO where you don't get access to the result of the winner until after cancelation of the loser completes, here we can provide the result immediately while canceling the loser concurrently. This is possible because we can hitch the backpressure on loser cancelation to the resource finalizer for the winner.
|
So the thing that makes me very nervous here is the fact that |
But what are race's semantics :) They do fall out naturally, if you don't know about resources. Specifically, that resources have a release phase. And |
|
Going to punt this to 3.4.1 so I can think about it a bit more. :-) |
| poll(F.canceled) *> F.never[(Either[A, B], ExitCase => F[Unit])] | ||
| } | ||
| } | ||
| case Right((f, oc)) => |
There was a problem hiding this comment.
I wonder if the code for the Left and Right cases could be shared and extracted to a common function? 🙂
| @@ -303,7 +303,67 @@ sealed abstract class Resource[F[_], +A] extends Serializable { | |||
| def race[B]( | |||
| that: Resource[F, B] | |||
| )(implicit F: Concurrent[F]): Resource[F, Either[A, B]] = | |||
| Concurrent[Resource[F, *]].race(this, that) | |||
| Resource.applyFull { poll => | |||
There was a problem hiding this comment.
Might be worth it to describe loser release behaviour (eager and concurrent) in the scaladoc, similarly to how it's done for the Resource#both. 🙂
There was a problem hiding this comment.
This genuinely makes me sad, but ultimately we don't have any type linearity to lean on to do any better.
For posterity, the argument here effectively reduces to the fourth case in join's finalization semantics:
/*
* 1. If the scope containing the *fiber* terminates:
* a) if the fiber is incomplete, run its inner finalizers when it completes
* b) if the fiber is succeeded, run its finalizers
* 2. If the fiber is canceled or errored, finalize
* 3. If the fiber succeeds and .cancel won the race, finalize eagerly and
* `join` results in `Canceled()`
* 4. If the fiber succeeds and .cancel lost the race or wasn't called,
* finalize naturally when the containing scope ends, `join` returns
* the value
*/An alternative choice that could have been made here is that the first join becomes the primary owner of the scope. If we had that semantic, then simply swapping *> for !> in the implementation of race (in GenSpawn) would have resolved this issue entirely, since then the losing join scope would be closed before the race itself returns.
IIRC, @wemrysi and I actually explored this a bit when we designed this semantic. A lot of the problems with it stem from the fact that Fiber is effectively a sort of Deferred, memoizing its result. Put more bluntly: you can call join repeatedly, and we don't really have any idea if the first one is the only one or the last one.
Since we have no guarantees here, Resource#start takes the safest possible approach, which involves a race condition between cancel and the inner fiber. This is deeply unfortunately, but it at least guarantees two properties: 1) you can never use after free, and 2) all results are guaranteed to be freeed eventually, though the exact timing on this free is nondeterministic in general. In a sense, this is the "garbage collector" tradeoff.
This PR effectively tightens up these guarantees in the special case of race.
Fixes #3111