typelevel / cats-effect

The pure asynchronous runtime for Scala
https://typelevel.org/cats-effect/
Apache License 2.0
1.99k stars 511 forks source link

Queued but unstarted work on `Dispatcher.sequential` should be cancelable #4069

Open mperucca opened 2 months ago

mperucca commented 2 months ago

A queued task to a dispatcher cannot be canceled:

import cats.effect.std.Dispatcher
import cats.effect.unsafe.implicits.global
import cats.effect.IO
import scala.concurrent.Await
import scala.concurrent.duration.Duration

val (dispatcher, shutDownDispatcher) = Dispatcher.sequential[IO].allocated.unsafeRunSync()

dispatcher.unsafeToFuture(IO.never)
val cancel = dispatcher.unsafeRunCancelable(IO.unit)
Await.result(cancel(), Duration.Inf) // hangs

Perhaps this is explained somewhere, but it was surprising to me.

armanbilge commented 2 months ago

https://github.com/typelevel/cats-effect/blob/769a89ef5d39f35d3a2cd00ffedbd22c91df48cc/std/shared/src/main/scala/cats/effect/std/Dispatcher.scala#L233-L235

If you need cancelation, use Dispatcher.parallel

mperucca commented 2 months ago

Ah, thanks. I was unknowingly reading the 3.5.3 scaladocs that my IDE had cached instead of the 3.5.4 ones. (Unfortunately Dispatcher.parallel doesn't fit our use case as we're replacing a cancelable actor model. The private Dispatcher.sequentialCancelable actually looks to have the behavior I was expecting though.)

armanbilge commented 2 months ago

Unfortunately Dispatcher.parallel doesn't fit our use case as we're replacing a cancelable actor model.

I see, can you say more about why it doesn't fit your usecase?

mperucca commented 2 months ago

We only want one action happening at a time in the order that Dispatcher.unsafe* is called, but we want any unstarted work to be cancelable. (We're migrating some code that had those semantics.)

armanbilge commented 2 months ago

We only want one action happening at a time in the order that Dispatcher.unsafe* is called

I see, are you calling Dispatcher.unsafe* from a single thread or multiple threads? which variant of unsafe* are you using?

armanbilge commented 2 months ago

but we want any unstarted work to be cancelable.

Ohhh I'm sorry I missed this subtlety. Actually I completely agree with you. This should be fixed. Canceling running work is challenging, but unqueued work should definitely be cancelable. I remember complaining about this too 🤔

Edit: aha, I think I raised this issue on Discord.