Cats-effect: IO's `Fiber#cancel` doesn't wait for finalizers if they already started running

[kubukoz]

In the following code:

val finalizer = IO.sleep(5.seconds) *> IO(println("finalized"))

val exhibitA = IO.unit.guarantee(finalizer).timeout(1.second)

val exhibitB = IO.never.guarantee(finalizer).timeout(1.second)

Sequencing exhibitA will complete after one second, without waiting for the finalizer to complete (it'll keep running in the background).

Sequencing exhibitB will wait one second, start the finalizer, wait for it to finish and only then continue.

I think this is a bug, as I was under the impression that cancel always waits for the finalizers.

I'd love to help fix this, if it's possible with the current implementation of IO (given some help).

ZIO handles the case properly but Monix Task also behaves like cats.effect.IO.

Tested on 2.0.0

[djspiewak]

Can definitely confirm that Fiber#cancel should always await the finalizer. This is sufficiently important that I almost want to make it a law:

bracket(F.unit)(_ => F.unit)(_ => F.never).start.flatMap(_.cancel) <-> F.never

[Avasil]

I remember @alexandru having some arguments against it but I don't remember where was the discussion

I personally agree that it should await the finalizer, it's easy to reverse behavior with cancel.start

[kubukoz]

@djspiewak how is never's equality defined?

[Avasil]

@djspiewak how is never's equality defined?

AFAIK it runs it and compares Future, ticks everything (I'm not sure if it shouldn't be something like ec.tick(1.day)) and checks the result. In case of IO.never, both values should be None

[kubukoz]

In that case I'm satisfied with the shape of that law :)

Alternatively,

bracket(acquire)(_ => F.never)(release).start.flatMap(_.cancel) <-> (acquire *> release)

plus a latch for waiting before cancel. I suppose something's wrong with my suggestion because there'll be some shifting involved before release, but I'm not sure.

[djspiewak]

I do like the more general law, though obviously it relies on the race condition without the latch (as does mine). We would probably do something like that.