typelevel / cats-effect

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

Dispatcher sequential runs queued tasks concurrently when closing #3981

Closed mperucca closed 4 months ago

mperucca commented 5 months ago

This might be specific to unsafeToFuture, but closing a Dispatcher runs all queued tasks and does so concurrently, which was even more alarming for sequential, as seen below.

val (dispatcher, cancel) = Dispatcher.sequential[IO].allocated.unsafeRunSync()
val buffer = ArrayBuffer[Int]()
for (i <- 0 to 99) {
  dispatcher.unsafeToFuture(IO(buffer += i))
}
cancel.unsafeRunSync() // buffer stays ordered if this line is removed (or the dispatcher finished already)

A test case in my code that initially hinted at this unexpected behavior was the following:

val (dispatcher, cancel) = Dispatcher.sequential[IO].allocated.unsafeRunSync()
dispatcher.unsafeToFuture(IO.never)
dispatcher.unsafeToFuture(IO.println("this runs when the dispatcher is closed"))
cancel.unsafeToFuture()

3.4.8 is the last version that behaves as expected.

This seems similar to https://github.com/typelevel/cats-effect/issues/3945, but the problem seemed to go farther back, possibly to https://github.com/typelevel/cats-effect/pull/3510.

armanbilge commented 5 months ago

Thanks for reporting! Note that we've got a significant rewrite of Dispatcher in flight which should hopefully fix this and many other bugs (but I have not verified this yet).

djspiewak commented 5 months ago

Wow this is amazing. I don't have any intuition about how this is happening, and I kinda don't want to. :P The rewrite definitely doesn't have this issue.