unsafeToFutureCancelable's cancel future completes before setting the result future's cancellation status

[mperucca]

trafficstars

The following fails fairly quickly:

while (true) {
  val (future, cancel) = IO.never.unsafeToFutureCancelable()
  Await.result(cancel(), Duration.Inf)
  assert(future.isCompleted)
}

I believe future actually has finished it's work, as the following doesn't fail:

while (true) {
  @volatile var started = false
  @volatile var done = false
  val io = IO { started = true }.bracket(_ => IO.never)(_ => IO { done = true })
  val (_, cancel) = io.unsafeToFutureCancelable()
  while (!started) {}
  Await.result(cancel(), Duration.Inf)
  assert(done)
}

This was mostly confusing in unit tests that were nondeterministic as they tested that the cancellation worked. I think it'd be more intuitive if the future's cancellation status was set before cancel completes, but maybe there's something else unintuitive that would result from that that I'm not considering.

[durban]

We could "fix" this, but then we'd have the opposite problem: the original Future would be isCompleted strictly before the cancel Future. I wonder which is more intuitive? The "most correct" would probably be to have them completed at exactly the same time. Except that's not really a thing with Futures. In theory we could try to do something with implementing Future directly... but honestly, I don't think it's worth it. I think isCompleted is just inherently racy...

[mperucca]

the original Future would be isCompleted strictly before the cancel Future

I assumed that's how it was intended to work, but perhaps it's less intuitive than I thought.

While pondering, I decided to try the following program that avoids Future altogether:

for {
  a <- IO.never.start
  winner <- a.join race a.cancel
} yield winner

Seems that sometimes the result is Left(Canceled()), sometimes it's Right(()), and sometimes it hangs. 🤔

[durban]

Uhh, that definitely shouldn't hang. It's fine that the result is sometimes Left/Right (race is nondeterministic), but if it hangs, that's a bug.

[durban]

@mperucca Could you provide the complete code that you see hanging? I tried, but couldn't get it to hang with the snippet above...

[mperucca]

Well I've been trying that snippet again in a loop (along with println) in a couple different versions of cats effect and Scala, and I can only reproduce the occasional hang when running on Scastie where it gets partway through, so I'd say it's just internet problems there. I'm real sorry for the false alarm.

I still think the original issue is surprising behavior, but maybe that's just me.

[durban]

Thanks for trying it again.

I was thinking about the original issue too, and I tend to agree, that the behavior you expected is the "more intuitive" one. Although, I'd like to see what others think...

[armanbilge]

I tend to agree, that the behavior you expected is the "more intuitive" one.

Me too. However, this is due to a race condition and is not easy to solve.

In theory we could try to do something with implementing Future directly... but honestly, I don't think it's worth it.

Can't say if it's worth it, but actually I think this is an interesting idea i.e. IOFiber[A] extends Future[A]. An IOFiber is a handle to a running task in the same way that a Future is. It would be pretty cool if we can "convert" to a Future simply by directly returning that handle. I started playing with this idea in 5b7bb92db8ec7577b5d019bd1904c216bc9d379f but don't have more time now to chase it further, maybe someone would be interested to pick it up :)