Skip to content

Commit 8934284

Browse files
authored
Merge pull request #280 from Dwolla/twitter-future-cancelation
Implement the Twitter Future cancellation protocol
2 parents 24c73e1 + 6d48a3d commit 8934284

3 files changed

Lines changed: 193 additions & 15 deletions

File tree

project/AsyncUtilsBuildPlugin.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,11 @@ object AsyncUtilsBuildPlugin extends AutoPlugin {
157157
libraryDependencies ++= {
158158
Seq(
159159
"org.typelevel" %% "cats-effect" % CatsEffect3V,
160+
"org.typelevel" %% "cats-effect-testkit" % CatsEffect3V,
160161
"com.twitter" %% "util-core" % v,
162+
"org.scalameta" %% "munit" % "1.2.1" % Test,
163+
"org.typelevel" %% "munit-cats-effect" % "2.1.0" % Test,
164+
"org.typelevel" %% "scalacheck-effect-munit" % "2.1.0-RC1" % Test,
161165
) ++ (if (scalaVersion.value.startsWith("2")) scala2CompilerPlugins else Nil)
162166
},
163167
mimaPreviousArtifacts += organizationName.value %% name.value % "0.3.0",
Lines changed: 47 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
package com.dwolla.util.async
22

3-
import cats._
4-
import cats.data._
5-
import cats.effect._
6-
import cats.syntax.all._
7-
import cats.tagless._
8-
import cats.tagless.syntax.all._
3+
import cats.*
4+
import cats.data.*
5+
import cats.effect.*
6+
import cats.syntax.all.*
7+
import cats.tagless.*
8+
import cats.tagless.syntax.all.*
9+
import com.dwolla.util.async.twitter.CancelledViaCatsEffect
910
import com.twitter.util
1011

12+
import java.util.concurrent.CancellationException
13+
import scala.util.control.NoStackTrace
14+
1115
object twitter extends ToAsyncFunctorKOps {
1216
implicit def twitterFutureAsyncFunctorK[F[_]]: util.Future ~~> F = new (util.Future ~~> F) {
1317
override def asyncMapK[Alg[_[_]] : FunctorK](alg: Alg[util.Future])
@@ -19,6 +23,10 @@ object twitter extends ToAsyncFunctorKOps {
1923
def provide[F[_]] = new PartiallyAppliedProvide[F]
2024

2125
def liftFuture[F[_]] = new PartiallyAppliedLiftFuture[F]
26+
27+
private[async] case object CancelledViaCatsEffect
28+
extends CancellationException("Cancelled via cats-effect")
29+
with NoStackTrace
2230
}
2331

2432
class PartiallyAppliedProvide[F[_]](private val dummy: Boolean = true) extends AnyVal {
@@ -34,15 +42,39 @@ class PartiallyAppliedProvide[F[_]](private val dummy: Boolean = true) extends A
3442
}
3543

3644
class PartiallyAppliedLiftFuture[F[_]] {
37-
def apply[A](fa: F[util.Future[A]])
38-
(implicit
39-
F: Async[F]): F[A] =
40-
Async[F].async[A] { cb =>
41-
fa.map {
42-
_.respond {
43-
case util.Return(a) => cb(Right(a))
44-
case util.Throw(ex) => cb(Left(ex))
45+
def apply[A](ffa: F[util.Future[A]])
46+
(implicit F: Async[F]): F[A] =
47+
MonadCancelThrow[F].uncancelable { (poll: Poll[F]) =>
48+
poll {
49+
Async[F].async[A] { cb: (Either[Throwable, A] => Unit) =>
50+
ffa
51+
.flatMap { fa =>
52+
Sync[F].delay {
53+
fa.respond {
54+
case util.Return(a) => cb(Right(a))
55+
case util.Throw(ex) => cb(Left(ex))
56+
}
57+
}
58+
}
59+
.map { fa =>
60+
Sync[F].delay {
61+
fa.raise(CancelledViaCatsEffect)
62+
}.some
63+
}
4564
}
46-
}.as(None)
65+
}
66+
.recoverWith(recoverFromCancelledViaCatsEffect)
4767
}
68+
69+
/**
70+
* According to CE maintainer Daniel Spiewak in Discord, there's
71+
* a race condition in the CE runtime that means sometimes it will
72+
* see the future as completed (with the `CancelledViaCatsEffect`
73+
* exception) before it transitions into the canceled state. This
74+
* `recoverWith` should prevent that from happening.
75+
*/
76+
private final def recoverFromCancelledViaCatsEffect[A](implicit F: Async[F]): PartialFunction[Throwable, F[A]] = {
77+
case CancelledViaCatsEffect =>
78+
Async[F].canceled >> Async[F].never
79+
}
4880
}
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
package com.dwolla.util.async
2+
3+
import cats.*
4+
import cats.effect.*
5+
import cats.effect.std.*
6+
import cats.effect.testkit.TestControl
7+
import cats.syntax.all.*
8+
import cats.effect.syntax.all.*
9+
import com.dwolla.util.async.twitter.{CancelledViaCatsEffect, liftFuture}
10+
import com.twitter.util.{Duration as _, *}
11+
import munit.{AnyFixture, CatsEffectSuite, ScalaCheckEffectSuite}
12+
import org.scalacheck.Arbitrary.arbitrary
13+
import org.scalacheck.effect.PropF
14+
import org.scalacheck.{Arbitrary, Gen, Prop, Test}
15+
16+
import java.util.concurrent.CancellationException
17+
import scala.concurrent.duration.*
18+
import scala.util.control.NoStackTrace
19+
20+
class TwitterFutureAsyncMapKTests extends CatsEffectSuite with ScalaCheckEffectSuite {
21+
override def munitIOTimeout: Duration = 1.minute
22+
23+
override protected def scalaCheckTestParameters: Test.Parameters =
24+
super.scalaCheckTestParameters.withMinSuccessfulTests(100000)
25+
26+
test("lift a Twitter Future into IO") {
27+
PropF.forAllF { (i: Int) =>
28+
for {
29+
promise <- IO(Promise[Int]())
30+
(x, _) <- liftFuture[IO](IO(promise)).both(IO(promise.setValue(i)))
31+
} yield {
32+
assertEquals(x, i)
33+
}
34+
}
35+
}
36+
37+
test("cancelling a running Twitter Future lifted into IO should interrupt the underlying Twitter Future") {
38+
for {
39+
promise <- IO(Promise[Int]())
40+
startedLatch <- CountDownLatch[IO](1)
41+
fiber <- IO.uncancelable { poll => // we want only the Future to be cancellable
42+
poll(liftFuture[IO](startedLatch.release.as(promise))).start
43+
}
44+
_ <- startedLatch.await
45+
_ <- fiber.cancel
46+
} yield {
47+
assert(promise.isInterrupted.isDefined)
48+
}
49+
}
50+
51+
private val supervisorAndDispatcher = ResourceTestLocalFixture("supervisorAndDispatcher",
52+
Supervisor[IO](await = true).product(Dispatcher.sequential[IO](await = true))
53+
)
54+
55+
override def munitFixtures: Seq[AnyFixture[?]] = super.munitFixtures ++ Seq(supervisorAndDispatcher)
56+
57+
test("a running Twitter Future lifted into IO can be completed (as success or failure) or cancelled") {
58+
PropF.forAllF { (i: Outcome[IO, Throwable, Int]) =>
59+
val (supervisor, dispatcher) = supervisorAndDispatcher()
60+
61+
TestControl.executeEmbed {
62+
for {
63+
expectedResult <- i.embed(CancelledViaCatsEffect.raiseError[IO, Int]).attempt
64+
capturedInterruptionThrowable <- Deferred[IO, Throwable]
65+
twitterPromise <- IO(new Promise[Int]()).flatTap(captureThrowableOnInterruption(dispatcher, capturedInterruptionThrowable))
66+
startedLatch <- CountDownLatch[IO](1)
67+
promiseFiber <- IO.uncancelable { poll => // we want only the Future to be cancellable
68+
supervisor.supervise(poll(liftFuture[IO](startedLatch.release.as(twitterPromise))))
69+
}
70+
_ <- startedLatch.await
71+
72+
(outcome, _) <- promiseFiber.join.both(completeOrCancel(i, twitterPromise, promiseFiber))
73+
74+
outcomeEmittedValue <- outcome.embed(CancelledViaCatsEffect.raiseError[IO, Int]).attempt
75+
76+
expectCancellation = i.isCanceled
77+
_ <- interceptMessageIO[CancellationException]("Cancelled via cats-effect") {
78+
capturedInterruptionThrowable
79+
.get
80+
.timeout(10.millis)
81+
.map(_.asLeft)
82+
.rethrow // interceptMessageIO works by throwing an exception, so we need to rethrow it to get the message
83+
}
84+
.whenA(expectCancellation)
85+
} yield {
86+
assertEquals(outcomeEmittedValue, expectedResult)
87+
assertEquals(outcome.isCanceled, i.isCanceled)
88+
assertEquals(Option(CancelledViaCatsEffect).filter(_ => expectCancellation), twitterPromise.isInterrupted)
89+
}
90+
}
91+
}
92+
}
93+
94+
// just here to make sure we understand how Twitter Future / Promise handles interruption
95+
test("the Twitter Future cancellation protocol") {
96+
Prop.forAll { (throwable: Throwable) =>
97+
val promise = Promise[Int]()
98+
99+
promise.raise(throwable)
100+
101+
assertEquals(promise.isInterrupted, throwable.some)
102+
}
103+
}
104+
105+
private def genOutcome[F[_] : Applicative, A: Arbitrary]: Gen[Outcome[F, Throwable, A]] =
106+
Gen.oneOf(
107+
arbitrary[A].map(_.pure[F]).map(Outcome.succeeded[F, Throwable, A]),
108+
Gen.const(new RuntimeException("arbitrary exception") with NoStackTrace).map(Outcome.errored[F, Throwable, A]),
109+
Gen.const(Outcome.canceled[F, Throwable, A]),
110+
)
111+
private implicit def arbOutcome[F[_] : Applicative, A: Arbitrary]: Arbitrary[Outcome[F, Throwable, A]] = Arbitrary(genOutcome)
112+
113+
private def captureThrowableOnInterruption[F[_] : Sync, A](dispatcher: Dispatcher[F],
114+
capture: Deferred[F, Throwable])
115+
(p: Promise[A]): F[Unit] =
116+
Sync[F].delay {
117+
p.setInterruptHandler { case ex =>
118+
dispatcher.unsafeRunSync(capture.complete(ex).void)
119+
}
120+
}
121+
122+
private def completeOrCancel[F[_] : Async, A](maybeA: Outcome[F, Throwable, A],
123+
promise: Promise[A],
124+
fiber: Fiber[F, Throwable, A]): F[Unit] =
125+
maybeA match {
126+
case Outcome.Succeeded(fa) =>
127+
fa.flatMap(a => Sync[F].delay(promise.setValue(a))).void
128+
129+
case Outcome.Errored(ex) =>
130+
// If the fiber is in the background (i.e. not joined) when it completes with an exception,
131+
// the IO runtime will print its stacktrace to stderr. We always plan to `join` the fiber
132+
// our tests are complete, so this feels like a false error report.
133+
134+
// To work around the issue, we delay the completion of the promise to make sure the fiber
135+
// is joined before the promise is completed with the exception.
136+
Sync[F].delay(promise.setException(ex)).delayBy(10.millis)
137+
138+
case Outcome.Canceled() =>
139+
fiber.cancel
140+
}
141+
142+
}

0 commit comments

Comments
 (0)