From 8da04079e259d43216a07a29d47b823022fcedbb Mon Sep 17 00:00:00 2001 From: Justin Reardon Date: Sat, 4 Jul 2026 21:45:22 -0400 Subject: [PATCH 1/3] Fix #4627 resource early cancelation Adds uncancelable wrapper to evaluation of resource so that cancelation can only occur at reasonable points. --- .../scala/cats/effect/kernel/Resource.scala | 128 +++++++++--------- .../scala/cats/effect/ResourceSuite.scala | 6 + 2 files changed, 72 insertions(+), 62 deletions(-) diff --git a/kernel/shared/src/main/scala/cats/effect/kernel/Resource.scala b/kernel/shared/src/main/scala/cats/effect/kernel/Resource.scala index b0614eda9f..4c225dda40 100644 --- a/kernel/shared/src/main/scala/cats/effect/kernel/Resource.scala +++ b/kernel/shared/src/main/scala/cats/effect/kernel/Resource.scala @@ -161,38 +161,43 @@ sealed abstract class Resource[F[_], +A] extends Serializable { case object Nil extends Stack[A] final case class Frame[AA, BB](head: AA => Resource[F, BB], tail: Stack[BB]) extends Stack[AA] - - // Indirection for calling `loop` needed because `loop` must be @tailrec - def continue[C](current: Resource[F, C], stack: Stack[C]): F[B] = - loop(current, stack) - - // Interpreter that knows how to evaluate a Resource data structure; - // Maintains its own stack for dealing with Bind chains - @tailrec def loop[C](current: Resource[F, C], stack: Stack[C]): F[B] = - current match { - case Allocate(resource) => - F.bracketFull(resource) { - case (a, _) => - stack match { - case Nil => onOutput(a) - case Frame(head, tail) => continue(head(a), tail) + F.uncancelable { poll => + // Indirection for calling `loop` needed because `loop` must be @tailrec + def continue[C](current: Resource[F, C], stack: Stack[C]): F[B] = + loop(current, stack) + + // Interpreter that knows how to evaluate a Resource data structure; + // Maintains its own stack for dealing with Bind chains + @tailrec def loop[C](current: Resource[F, C], stack: Stack[C]): F[B] = + current match { + case Allocate(resource) => + poll { + F.bracketFull(resource) { + case (a, _) => + stack match { + case Nil => onOutput(a) + case Frame(head, tail) => continue(head(a), tail) + } + } { + case ((_, release), outcome) => + onRelease(release, ExitCase.fromOutcome(outcome)) } - } { - case ((_, release), outcome) => - onRelease(release, ExitCase.fromOutcome(outcome)) - } - case Bind(source, fs) => - loop(source, Frame(fs, stack)) - case Pure(v) => - stack match { - case Nil => onOutput(v) - case Frame(head, tail) => - loop(head(v), tail) - } - case Eval(fa) => - fa.flatMap(a => continue(Resource.pure(a), stack)) - } - loop(this, Nil) + } + case Bind(source, fs) => + loop(source, Frame(fs, stack)) + case Pure(v) => + stack match { + case Nil => onOutput(v) + case Frame(head, tail) => + loop(head(v), tail) + } + case Eval(fa) => + poll(fa).flatMap(a => continue(Resource.pure(a), stack)) + } + + loop(this, Nil) + + } } /** @@ -467,23 +472,22 @@ sealed abstract class Resource[F[_], +A] extends Serializable { case object Nil extends Stack[B] final case class Frame[AA, BB](head: AA => Resource[F, BB], tail: Stack[BB]) extends Stack[AA] - - // Indirection for calling `loop` needed because `loop` must be @tailrec - def continue[C]( - current: Resource[F, C], - stack: Stack[C], - release: ExitCase => F[Unit]): F[(B, ExitCase => F[Unit])] = - loop(current, stack, release) - - // Interpreter that knows how to evaluate a Resource data structure; - // Maintains its own stack for dealing with Bind chains - @tailrec def loop[C]( - current: Resource[F, C], - stack: Stack[C], - release: ExitCase => F[Unit]): F[(B, ExitCase => F[Unit])] = - current match { - case Allocate(resource) => - F uncancelable { poll => + F uncancelable { poll => + // Indirection for calling `loop` needed because `loop` must be @tailrec + def continue[C]( + current: Resource[F, C], + stack: Stack[C], + release: ExitCase => F[Unit]): F[(B, ExitCase => F[Unit])] = + loop(current, stack, release) + + // Interpreter that knows how to evaluate a Resource data structure; + // Maintains its own stack for dealing with Bind chains + @tailrec def loop[C]( + current: Resource[F, C], + stack: Stack[C], + release: ExitCase => F[Unit]): F[(B, ExitCase => F[Unit])] = + current match { + case Allocate(resource) => resource(poll) flatMap { case (b, rel) => // Insert F.unit to emulate defer for stack-safety @@ -510,24 +514,24 @@ sealed abstract class Resource[F[_], +A] extends Serializable { .onError { case e => rel(ExitCase.Errored(e)).handleError(_ => ()) } } } - } - case Bind(source, fs) => - loop(source, Frame(fs, stack), release) + case Bind(source, fs) => + loop(source, Frame(fs, stack), release) - case Pure(v) => - stack match { - case Nil => - (v: B, release).pure[F] - case Frame(head, tail) => - loop(head(v), tail, release) - } + case Pure(v) => + stack match { + case Nil => + (v: B, release).pure[F] + case Frame(head, tail) => + loop(head(v), tail, release) + } - case Eval(fa) => - fa.flatMap(a => continue(Resource.pure(a), stack, release)) - } + case Eval(fa) => + poll(fa).flatMap(a => continue(Resource.pure(a), stack, release)) + } - loop(this, Nil, _ => F.unit) + loop(this, Nil, _ => F.unit) + } } /** diff --git a/tests/shared/src/test/scala/cats/effect/ResourceSuite.scala b/tests/shared/src/test/scala/cats/effect/ResourceSuite.scala index 873c1004c0..48a48a6a7e 100644 --- a/tests/shared/src/test/scala/cats/effect/ResourceSuite.scala +++ b/tests/shared/src/test/scala/cats/effect/ResourceSuite.scala @@ -154,6 +154,12 @@ class ResourceSuite extends BaseScalaCheckSuite with DisciplineSuite { forAll { (fa: IO[String]) => assertEqv(Resource.eval(fa).use(IO.pure), fa) } } + real("eval - uncancelable timeout is not canceled") { + val test = Resource.eval(IO.uncancelable(_ => IO.sleep(100.millis))).timeout(10.millis).use_ + test + + } + ticked("eval - interruption") { implicit ticker => def resource(d: Deferred[IO, Int]): Resource[IO, Unit] = for { From 81c246dae7c54b415e639f2b3042cc0aedbbeb2d Mon Sep 17 00:00:00 2001 From: Justin Reardon Date: Sun, 5 Jul 2026 20:21:50 -0400 Subject: [PATCH 2/3] narrow usages of poll in Resource - polling the entire brackFull unmasks the rest of the evaluation. Instead we have to just mask the acquire and use - in allocatedCase, insert cancelation boundary before checking the next frame instead of unmasking the rest of the evaluation --- .../scala/cats/effect/kernel/Resource.scala | 24 +++++++++---------- .../scala/cats/effect/ResourceSuite.scala | 20 ++++++++++++++-- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/kernel/shared/src/main/scala/cats/effect/kernel/Resource.scala b/kernel/shared/src/main/scala/cats/effect/kernel/Resource.scala index 4c225dda40..f29fb5f03f 100644 --- a/kernel/shared/src/main/scala/cats/effect/kernel/Resource.scala +++ b/kernel/shared/src/main/scala/cats/effect/kernel/Resource.scala @@ -171,23 +171,21 @@ sealed abstract class Resource[F[_], +A] extends Serializable { @tailrec def loop[C](current: Resource[F, C], stack: Stack[C]): F[B] = current match { case Allocate(resource) => - poll { - F.bracketFull(resource) { - case (a, _) => - stack match { - case Nil => onOutput(a) - case Frame(head, tail) => continue(head(a), tail) - } - } { - case ((_, release), outcome) => - onRelease(release, ExitCase.fromOutcome(outcome)) - } + F.bracketFull(p => p(resource(poll))) { + case (a, _) => + stack match { + case Nil => poll(onOutput(a)) + case Frame(head, tail) => continue(head(a), tail) + } + } { + case ((_, release), outcome) => + onRelease(release, ExitCase.fromOutcome(outcome)) } case Bind(source, fs) => loop(source, Frame(fs, stack)) case Pure(v) => stack match { - case Nil => onOutput(v) + case Nil => poll(onOutput(v)) case Frame(head, tail) => loop(head(v), tail) } @@ -509,7 +507,7 @@ sealed abstract class Resource[F[_], +A] extends Serializable { F.pure((b, rel2)) case Frame(head, tail) => - poll(continue(head(b), tail, rel2)) + (poll(F.unit) >> continue(head(b), tail, rel2)) .onCancel(rel(ExitCase.Canceled)) .onError { case e => rel(ExitCase.Errored(e)).handleError(_ => ()) } } diff --git a/tests/shared/src/test/scala/cats/effect/ResourceSuite.scala b/tests/shared/src/test/scala/cats/effect/ResourceSuite.scala index 48a48a6a7e..0f01a978d6 100644 --- a/tests/shared/src/test/scala/cats/effect/ResourceSuite.scala +++ b/tests/shared/src/test/scala/cats/effect/ResourceSuite.scala @@ -39,6 +39,8 @@ import munit.DisciplineSuite class ResourceSuite extends BaseScalaCheckSuite with DisciplineSuite { + override def scalaCheckInitialSeed = "EpTk-jCEjNXrCuelFnCg7QRFmJK5gqhF6DEW9EUaFtF=" + private implicit def resourceShow[A]: Show[Resource[IO, A]] = Show.fromToString tickedProperty("releases resources in reverse order of acquisition") { implicit ticker => @@ -155,9 +157,23 @@ class ResourceSuite extends BaseScalaCheckSuite with DisciplineSuite { } real("eval - uncancelable timeout is not canceled") { - val test = Resource.eval(IO.uncancelable(_ => IO.sleep(100.millis))).timeout(10.millis).use_ - test + Resource.eval(IO.uncancelable(_ => IO.sleep(100.millis))).timeout(10.millis).use_ + } + + real("eval - uncancelable continuation") { + val res = Resource + .make(IO.pure(42))(_ => IO.unit) + .flatMap(_ => Resource.eval(IO.uncancelable { _ => IO.canceled })) + for { + ctr <- IO.ref(0) + fib <- IO.uncancelable { poll => + poll(res.allocatedCase).flatMap { _ => ctr.update(_ + 1) } + }.start + _ <- fib.join + c <- ctr.get + _ <- IO { assertEquals(c, 1) } + } yield () } ticked("eval - interruption") { implicit ticker => From 461008f4f245da974ed0e6f364ec77f5ad2b807e Mon Sep 17 00:00:00 2001 From: Justin Reardon Date: Sat, 18 Jul 2026 21:46:49 -0400 Subject: [PATCH 3/3] Resource orElse is not like combineK in Resource, combineK has a bespoke implementation which propagates the final exit case to both inner resource finalizers, while orElse is derived from handleErrorWith, which does not propagate the final exit case, so it's not surprising that the test that it is equivalent to combineK would fail. Removing this test. --- .../src/test/scala/cats/effect/ResourceSuite.scala | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/tests/shared/src/test/scala/cats/effect/ResourceSuite.scala b/tests/shared/src/test/scala/cats/effect/ResourceSuite.scala index 0f01a978d6..46f5a2595c 100644 --- a/tests/shared/src/test/scala/cats/effect/ResourceSuite.scala +++ b/tests/shared/src/test/scala/cats/effect/ResourceSuite.scala @@ -39,8 +39,6 @@ import munit.DisciplineSuite class ResourceSuite extends BaseScalaCheckSuite with DisciplineSuite { - override def scalaCheckInitialSeed = "EpTk-jCEjNXrCuelFnCg7QRFmJK5gqhF6DEW9EUaFtF=" - private implicit def resourceShow[A]: Show[Resource[IO, A]] = Show.fromToString tickedProperty("releases resources in reverse order of acquisition") { implicit ticker => @@ -744,16 +742,6 @@ class ResourceSuite extends BaseScalaCheckSuite with DisciplineSuite { assertEquals(released, acquired) } - tickedProperty("combineK - behave like orElse when underlying effect does") { - implicit ticker => - forAll { (r1: Resource[IO, Int], r2: Resource[IO, Int]) => - val lhs = r1.orElse(r2) - val rhs = r1 <+> r2 - - assertEqv(lhs, rhs) - } - } - tickedProperty("combineK - behave like underlying effect") { implicit ticker => forAll { (ot1: OptionT[IO, Int], ot2: OptionT[IO, Int]) => val lhs = Resource.eval(ot1 <+> ot2).use(OptionT.pure[IO](_)).value