Cats-effect: IORunLoop loops indefinitely after unsafeRunSync is called

[entangled90]

The following snippet of code after is being run continues to utilize 1 cpu (100% usage):

import cats.effect.IO
import cats.effect.internals.IOContextShift
import cats.implicits._
import scala.concurrent.duration._

import scala.concurrent.ExecutionContext
implicit val ec = IOContextShift.global

implicit val timer = IO.timer(ExecutionContext.Implicits.global)

val f = for {
  fiber <- IO("success").foreverM.start
  _     <- fiber.cancel
} yield "success"

IO.race(f, IO.sleep(200 millis) *> IO("failed")).unsafeRunSync() == Left("success")

As other have pointed out in gitter, IO("success") is not cancelable, but IO("success").foreverM should be.

I profiled the application (run inside sbt test) with yourkit and this is the relevant part:

[SystemFw]

What version are you on? RC3 since you have ContextShift

[mpilquist]

What if you write f this way instead?

IO("success").foreverM.start.bracket(_ => IO.pure("success"))(_.cancel)

[alexandru]

What does foreverM do?

As other have pointed out in gitter, IO("success") is not cancelable, but IO("success").foreverM should be.

Not really, without any async or "cancel" boundaries inserted in such a loop, it isn't. I mean the background process that does the processing, not talking about the bind continuation of the result.

Note that that race will create a race condition and ignore the result of your loop afterwards, but that loop continues to run on another thread.

[SystemFw]

Not really, without any async or "cancel" boundaries inserted in such a loop, it isn't.

Shouldn't it be with auto-cancellable binds?

[alexandru]

Yes, but that flag is checked only on async boundaries (e.g. stuff built with IO.async).

[SystemFw]

Yes, but that flag is checked only on async boundaries (e.g. stuff built with IO.async).

That is very surprising. I would expect that snippet to work. Otherwise what is the difference with non-auto-cancellable-binds?

[SystemFw]

foreverM == fa.flatMap(_ => fa) (but stack-safe for all monads due to tailRecM)

[alexandru]

Common sense in this case is not reliable. Most people expect magic in the case of cancelation.

Even if you take care of this case:

1. it's useless; obviously doing never-ending loops without async boundaries in it is a bad idea and not just for cancelability reasons
   
   
   
   • being able to interrupt something involves fairness; in absence of fairness it's not reasonable to expect interruption
   
   
2. this does not take care of actual while loops or other JVM-specific logic, for which you'd need a Thread.interrupt

We need to draw lines and the line was currently drawn at async boundaries that we manage.

[alexandru]

Also, fix is easy:

1. with near-zero overhead: (IO.cancelBoundary *> task).foreverM
2. with forking: (IO.shift *> task).foreverM

[SystemFw]

it's useless; obviously doing never-ending loops without async boundaries in it is a bad idea and not just for cancelability reasons

I guess I'm unclear on what exactly constitutes an async boundary in this case. Having some Timer.sleep operation would make that cancellable? How about waiting on an MVar? I'm not interested in this exact code but in real-world scenarios that resemble it, and perhaps those work :)

being able to interrupt something involves fairness; in absence of fairness it's not reasonable to expect interruption

This is a good point

this does not care of actual while loops or other JVM-specific logic, for which you'd need a Thread.interrupt

I don't buy into this argument in general, use flatMap chains instead of while, it's a best effort thing

[SystemFw]

Wouldn't it make sense to have cancelBoundary in a type class btw?

[SystemFw]

Having some Timer.sleep operation would make that cancellable? How about waiting on an MVar?

I'm actually expecting these to work, the snippet above can't really be made to work without fairness (e.g. Fibers being scheduled by the IO interpreter "manually" or with auto-shifting), since it never yields back control. I do think cancelBoundary should be polymorphic, but perhaps it can't be supported by every data type out there?

EDIT: well, without fairness it can be made to work with best-effort guarantees, but not anything stronger than that

[alexandru]

I guess I'm unclear on what exactly constitutes an async boundary in this case.

Everything that gets created via IO.async, IO.asyncF, IO.cancelable or IO.bracket.

And obviously anything that involves a shift or cancelBoundary, which are created via IO.async themselves.

Having some Timer.sleep operation would make that cancellable?

Yes, that's cancelable.

How about waiting on an MVar?

Yes, that's cancelable.

Wouldn't it make sense to have cancelBoundary in a type class btw?

Maybe, I've gone back and forth on it myself. I don't think there are many cases in which it is needed — besides the never ending, synchronous loops which frankly I haven't seen much in actual code. But it does well in demos that calculate Fibonacci 🙂

If we decide on it, it will have to wait for 2.0 though. 1.0.0 is next Monday and this is not the kind of emergency that can delay it.

[entangled90]

@mpilquist your snippets has the same issues as mine.
Adding a cancel boundary or forking the process fixes it.
I Guess you can close it. I did not have a clear idea of how cancellation works. Thanks!

[SystemFw]

@alexandru I actually agree with you 100%

I saw the snippet and I unconsciously mapped it to things that actually do have an async boundary, without realising that that's the key thing that makes this behaviour happen.

[Avasil]

Seems like this section is out of date and could use refactoring before release to avoid confusion