rssh / cps-async-connect

Apache License 2.0
17 stars 3 forks source link

Behavior of finalizers #9

Open alexandru opened 2 months ago

alexandru commented 2 months ago

Hey 👋

//> using scala "3.5.1"
//> using dep "org.typelevel::cats-effect:3.5.4"
//> using dep "com.github.rssh::cps-async-connect-cats-effect-loom:0.9.22"

import cps._
import cps.monads.catsEffect.given
import cats.syntax.all.given
import cats.effect.*

object Main extends IOApp.Simple:
    def run = async[IO]:
        try 
            println("Starting!")
            IO.canceled
              .onCancel(IO.println("Trying to finalize..."))
              .await
        finally 
            println("Finalizing!") 

The code under finally does not execute.

In Cats-Effect, the cancellation isn't an exception, but rather a signal that's unwinding the stack on its own special channel. So for example, the code catch does not execute here and that's fine. And obviously, people shouldn't catch InterruptedException in Java, either, most of the time.

try 
    IO.canceled.await
catch
    case e: Throwable =>
        println("Caught cancellation?") 

But I'm thinking that maybe finally should execute 🤷‍♂️

So, this code:

try 
    println("Starting!")
    IO.canceled.await
finally
    println("Finalizing!") 

Could it be translated to the equivalent of this code?

IO.defer {
    println("Starting!")
    IO.canceled
}.guarantee {
    IO(println("Finalizing!"))
}

Note that I'm not 100% if this would be a good design.


Thanks for this library 🙏

rssh commented 2 months ago

The situation with MonadCancel and MonadError is exactly as with Iterator and CloseableIterator in our reddit discussion ;) I.e., MonadCancel extends MonadError but breaks its protocol.

Sure, we can rewrite operations for MonadCancel. It's require some decisions about behaviour:

try {
    IO.canceled.await
} catch {
    case ex: CancellationException =>
         println("cancellation catch")
} finally {
     println("finalize")  
}. 

Are we want printing "cancellation catch" ? (In spirit of cats-effect - no, in spirit of Java compability - yes ...) or raise a warning....

rssh commented 2 months ago

What is the 'IO' way to deal with exceptions in finalizers during handling cancel? I.e., we have the next code:

val run = async[IO] {
      try {
        IO.canceled.await
      } finally {
        val x = await(IO.delay(1))
        throw new RuntimeException("AAA")
      }

In the semantics of 'normal'. java, the result should be throwing RuntimeException. In. IO we have CancellableException, and we have nothing even in ex.getSupressed() after unsafeRun.

alexandru commented 2 months ago

Are we want printing "cancellation catch" ? (In spirit of cats-effect - no, in spirit of Java compability - yes ...) or raise a warning....

The problem with catching, Java-style, is that it provides the ability to ignore cancellation, and that's something that Cats-Effect doesn't allow by design. Once interruption of a fiber has started, it can't be stopped.

Note that in Java, people usually catch Exception instead of Throwable; otherwise they risk catching Error instances, which are often virtual machine errors, such as java.lang.OutOfMemoryError. So even in Java there are exceptions that should never be caught, although this is enforced by conventions and linting. But the finalizers always get called, which is why I feel this would be the right call.

try {
  // ...
} catch (Exception e) {
  // Doesn't catch everything
  // ...
} finally {
  // Always gets called
  println("Finalizing");
}

BTW, I don't have much experience with ZIO, but I'm pretty sure this is ZIO's design as well; meaning that once interruption starts, it can't be caught and ignored. Happy to be proven wrong.

In other words, we don't necessarily want Java compatibility because it's pretty hard to do, given the current design of Cats-Effect and this design happened because Java's interruption protocol is flawed.

alexandru commented 2 months ago

What is the 'IO' way to deal with exceptions in finalizers during handling cancel?

Cancellation is a concurrent operation, meaning that you have one fiber cancelling another fiber.

for {
  fiber <- task.start
  _ <- IO.sleep(3.seconds)
  // Fiber runs concurrently, but we cancel it 
  // and wait for its termination
  _ <- fiber.cancel  
} yield ()

That cancel will finish when the fiber gets terminated; however, it won't signal an outcome. And then you can do a fiber.join, which can only tell you that the fiber was cancelled. In other words, the main fiber can't get exceptions that occurred during the cancellation.

I'll have to check, but I remember that those exceptions are getting logged via the ExecutionContext in IORuntime. Note that it doesn't crash the stack of finalizers that have to be executed.

IO.canceled.guarantee {
    IO.raiseError(new RuntimeException("Boom! (1)"))
}.guarantee {
    IO.println("OK! (1)")
}.guarantee {
    IO.raiseError(new RuntimeException("Boom! (2)"))
}.guarantee {
    IO.println("OK! (2)")
}

This works much like the following, except that exceptions simply get logged, instead of being accumulated in as “suppressed” exceptions.

try {
    try {
        try {
            try {
                throw new InterruptedException()
            } finally {
                throw new RuntimeException("Boom! (1)")
            }
        } finally {
            println("OK! (1)")
        }
    } finally {
        throw new RuntimeException("Boom! (2)")
    }
} finally {
    println("OK! (2)")
} 
rssh commented 2 months ago

Thanks. Ok, now I can say that we have an initial implementation here: https://github.com/rssh/cps-async-connect/blob/guarantee-in-finally/cats-effect/shared/src/main/scala/cps/monads/catsEffect/CatsAsync.scala

test suite (https://github.com/rssh/cps-async-connect/blob/guarantee-in-finally/cats-effect/shared/src/test/scala/cps/catsEffect/TryFinallyCancellableSuite.scala ) is in progress

rssh commented 2 months ago

The non-trivial part will be the documentation explaining the differences between computation models.

rssh commented 1 month ago

Yet one problem: behavior of guaranteeCase when an exception is thrown inside the first argument. I.e. next code:

async[IO] {
  try {
      await(IO.raiseError(new RuntimeException("BBB")))
  }
} finally {
   throw new RuntimeException("AAA")
}

will report RuntimeException("BBB") instead of "AAA" (as in plain Java). It is better to have traditional behavior in this case.