Transactor.transact wraps the user's body in try ... catch case t =>, which matches every Throwable — including fatal ones like OutOfMemoryError, StackOverflowError, and other VirtualMachineErrors. On a fatal error the code then attempts con.rollback() on a connection whose state is, by definition, undefined. That call can hang, mask the original error, or raise a secondary error that gets only added as suppressed.
Reproducer (illustrative — same control flow without a real DB):
//> using scala 3.8.3
import scala.util.control.NonFatal
@main def run(): Unit =
// Magnum's pattern: catches everything
def magnumStyle[T](f: => T): T =
try f
catch case t =>
// would call con.rollback() here on a corrupted connection
println(s"caught fatal: ${t.getClass.getSimpleName}")
throw t
try magnumStyle(throw new StackOverflowError("simulated"))
catch case _: StackOverflowError => ()
Source:
|
try |
|
val res = f(using DbTx(con, sqlLogger)) |
|
con.commit() |
|
res |
|
catch |
|
case t => |
|
try con.rollback() |
|
catch { case t2 => t.addSuppressed(t2) } |
|
throw t |
try
val res = f(using DbTx(con, sqlLogger))
con.commit()
res
catch
case t =>
try con.rollback()
catch { case t2 => t.addSuppressed(t2) }
throw t
The standard Scala convention is case NonFatal(t) =>, which lets VirtualMachineError, ThreadDeath, LinkageError, etc. propagate without trying to do recovery on a JVM that may already be doomed. In a transactor specifically this matters because con.rollback() on OutOfMemoryError can itself try to allocate, which then deadlocks or compounds the failure.
Suggested fix:
catch
case NonFatal(t) =>
try con.rollback()
catch { case NonFatal(t2) => t.addSuppressed(t2) }
throw t
Happy to PR.
Transactor.transactwraps the user's body intry ... catch case t =>, which matches everyThrowable— including fatal ones likeOutOfMemoryError,StackOverflowError, and otherVirtualMachineErrors. On a fatal error the code then attemptscon.rollback()on a connection whose state is, by definition, undefined. That call can hang, mask the original error, or raise a secondary error that gets only added as suppressed.Reproducer (illustrative — same control flow without a real DB):
Source:
magnum/magnum/src/main/scala/com/augustnagro/magnum/Transactor.scala
Lines 27 to 35 in 2801364
The standard Scala convention is
case NonFatal(t) =>, which letsVirtualMachineError,ThreadDeath,LinkageError, etc. propagate without trying to do recovery on a JVM that may already be doomed. In a transactor specifically this matters becausecon.rollback()onOutOfMemoryErrorcan itself try to allocate, which then deadlocks or compounds the failure.Suggested fix:
Happy to PR.