From 66686c3afa9bab1e3685cf8a89bf060e0e6e3dca Mon Sep 17 00:00:00 2001 From: Patrik Nordwall Date: Mon, 25 Nov 2019 11:57:15 +0100 Subject: [PATCH] Use stop supervision for PropsAdapter, #28035 (#28182) * scaladsl.PropsAdapter and javadsl.Adapter.props should use stop supervision, otherwise it will restart (from classic) by default * PropsAdapter is used by Cluster Singleton and Sharding * also typed.ActorSystem.systemActorOf * removed default param value in internal.PropsAdapter to make the decision of rethrowTypedFailure more explicit * rethrowTypedFailure=true is used for the child faild propagation to parent and only makes sense when the parent is Typed --- .../ClassicSupervisingTypedSpec.scala | 20 +++++++++++++++++++ .../internal/adpater/PropsAdapterSpec.scala | 2 +- .../issue-28035-supervision.excludes | 3 +++ .../scala/akka/actor/typed/ActorSystem.scala | 3 ++- .../internal/adapter/ActorSystemAdapter.scala | 9 ++++++++- .../typed/internal/adapter/PropsAdapter.scala | 19 ++++++++---------- .../akka/actor/typed/javadsl/Adapter.scala | 6 +++++- .../typed/scaladsl/adapter/PropsAdapter.scala | 7 ++++++- 8 files changed, 53 insertions(+), 16 deletions(-) create mode 100644 akka-actor-typed/src/main/mima-filters/2.6.0.backwards.excludes/issue-28035-supervision.excludes diff --git a/akka-actor-typed-tests/src/test/scala/akka/actor/typed/coexistence/ClassicSupervisingTypedSpec.scala b/akka-actor-typed-tests/src/test/scala/akka/actor/typed/coexistence/ClassicSupervisingTypedSpec.scala index d5be0fba42..eb82b5bd37 100644 --- a/akka-actor-typed-tests/src/test/scala/akka/actor/typed/coexistence/ClassicSupervisingTypedSpec.scala +++ b/akka-actor-typed-tests/src/test/scala/akka/actor/typed/coexistence/ClassicSupervisingTypedSpec.scala @@ -126,6 +126,26 @@ class ClassicSupervisingTypedSpec extends WordSpecLike with LogCapturing with Be probe.expectNoMessage() expectTerminated(underTest.toClassic) } + + "default to stop for supervision of systemActorOf" in { + val probe = TestProbe() + val underTest = classicSystem.toTyped.systemActorOf(ProbedBehavior.behavior(probe.ref), "s1") + watch(underTest.toClassic) + underTest ! "throw" + probe.expectMsg(PostStop) + probe.expectNoMessage() + expectTerminated(underTest.toClassic) + } + + "default to stop for PropsAdapter" in { + val probe = TestProbe() + val underTest = classicSystem.actorOf(PropsAdapter(ProbedBehavior.behavior(probe.ref))) + watch(underTest) + underTest ! "throw" + probe.expectMsg(PostStop) + probe.expectNoMessage() + expectTerminated(underTest) + } } override protected def afterAll(): Unit = { diff --git a/akka-actor-typed-tests/src/test/scala/akka/actor/typed/internal/adpater/PropsAdapterSpec.scala b/akka-actor-typed-tests/src/test/scala/akka/actor/typed/internal/adpater/PropsAdapterSpec.scala index 4ab1b180ae..ceefdeccf5 100644 --- a/akka-actor-typed-tests/src/test/scala/akka/actor/typed/internal/adpater/PropsAdapterSpec.scala +++ b/akka-actor-typed-tests/src/test/scala/akka/actor/typed/internal/adpater/PropsAdapterSpec.scala @@ -16,7 +16,7 @@ class PropsAdapterSpec extends WordSpec with Matchers { "PropsAdapter" should { "default to akka.dispatch.SingleConsumerOnlyUnboundedMailbox" in { val props: Props = Props.empty - val pa: actor.Props = PropsAdapter(() => Behaviors.empty, props) + val pa: actor.Props = PropsAdapter(() => Behaviors.empty, props, rethrowTypedFailure = false) pa.mailbox shouldEqual "akka.actor.typed.default-mailbox" } } diff --git a/akka-actor-typed/src/main/mima-filters/2.6.0.backwards.excludes/issue-28035-supervision.excludes b/akka-actor-typed/src/main/mima-filters/2.6.0.backwards.excludes/issue-28035-supervision.excludes new file mode 100644 index 0000000000..381a030498 --- /dev/null +++ b/akka-actor-typed/src/main/mima-filters/2.6.0.backwards.excludes/issue-28035-supervision.excludes @@ -0,0 +1,3 @@ +# #28035 default to stop supervision when spawning via PropsAdapter + +ProblemFilters.exclude[DirectMissingMethodProblem]("akka.actor.typed.internal.adapter.PropsAdapter.apply*") diff --git a/akka-actor-typed/src/main/scala/akka/actor/typed/ActorSystem.scala b/akka-actor-typed/src/main/scala/akka/actor/typed/ActorSystem.scala index d938fcd489..52d41fc0d1 100644 --- a/akka-actor-typed/src/main/scala/akka/actor/typed/ActorSystem.scala +++ b/akka-actor-typed/src/main/scala/akka/actor/typed/ActorSystem.scala @@ -262,7 +262,8 @@ object ActorSystem { appConfig, cl, executionContext, - Some(PropsAdapter[Any](() => GuardianStartupBehavior(guardianBehavior), guardianProps)), + Some( + PropsAdapter[Any](() => GuardianStartupBehavior(guardianBehavior), guardianProps, rethrowTypedFailure = false)), setup) system.start() diff --git a/akka-actor-typed/src/main/scala/akka/actor/typed/internal/adapter/ActorSystemAdapter.scala b/akka-actor-typed/src/main/scala/akka/actor/typed/internal/adapter/ActorSystemAdapter.scala index c372eca4d4..3d1730d54d 100644 --- a/akka-actor-typed/src/main/scala/akka/actor/typed/internal/adapter/ActorSystemAdapter.scala +++ b/akka-actor-typed/src/main/scala/akka/actor/typed/internal/adapter/ActorSystemAdapter.scala @@ -22,6 +22,7 @@ import akka.actor.typed.Dispatchers import akka.actor.typed.Props import akka.actor.typed.Scheduler import akka.actor.typed.Settings +import akka.actor.typed.SupervisorStrategy import akka.actor.typed.internal.ActorRefImpl import akka.actor.typed.internal.ExtensionsImpl import akka.actor.typed.internal.InternalRecipientRef @@ -29,6 +30,7 @@ import akka.actor.typed.internal.PropsImpl.DispatcherDefault import akka.actor.typed.internal.PropsImpl.DispatcherFromConfig import akka.actor.typed.internal.PropsImpl.DispatcherSameAsParent import akka.actor.typed.internal.SystemMessage +import akka.actor.typed.scaladsl.Behaviors import akka.annotation.InternalApi import akka.{ actor => classic } import org.slf4j.{ Logger, LoggerFactory } @@ -107,7 +109,12 @@ import org.slf4j.{ Logger, LoggerFactory } FutureConverters.toJava(whenTerminated) override def systemActorOf[U](behavior: Behavior[U], name: String, props: Props): ActorRef[U] = { - val ref = system.systemActorOf(PropsAdapter(() => behavior, props), name) + val ref = system.systemActorOf( + PropsAdapter( + () => Behaviors.supervise(behavior).onFailure(SupervisorStrategy.stop), + props, + rethrowTypedFailure = false), + name) ActorRefAdapter(ref) } diff --git a/akka-actor-typed/src/main/scala/akka/actor/typed/internal/adapter/PropsAdapter.scala b/akka-actor-typed/src/main/scala/akka/actor/typed/internal/adapter/PropsAdapter.scala index dd5da04a89..72ad0e77be 100644 --- a/akka-actor-typed/src/main/scala/akka/actor/typed/internal/adapter/PropsAdapter.scala +++ b/akka-actor-typed/src/main/scala/akka/actor/typed/internal/adapter/PropsAdapter.scala @@ -18,19 +18,16 @@ import akka.dispatch.Mailboxes * INTERNAL API */ @InternalApi private[akka] object PropsAdapter { - def apply[T]( - behavior: () => Behavior[T], - deploy: Props = Props.empty, - rethrowTypedFailure: Boolean = true): akka.actor.Props = { - val props = akka.actor.Props(new ActorAdapter(behavior(), rethrowTypedFailure)) + def apply[T](behavior: () => Behavior[T], props: Props, rethrowTypedFailure: Boolean): akka.actor.Props = { + val classicProps = akka.actor.Props(new ActorAdapter(behavior(), rethrowTypedFailure)) - val dispatcherProps = (deploy.firstOrElse[DispatcherSelector](DispatcherDefault.empty) match { - case _: DispatcherDefault => props - case DispatcherFromConfig(name, _) => props.withDispatcher(name) - case _: DispatcherSameAsParent => props.withDispatcher(Deploy.DispatcherSameAsParent) + val dispatcherProps = (props.firstOrElse[DispatcherSelector](DispatcherDefault.empty) match { + case _: DispatcherDefault => classicProps + case DispatcherFromConfig(name, _) => classicProps.withDispatcher(name) + case _: DispatcherSameAsParent => classicProps.withDispatcher(Deploy.DispatcherSameAsParent) }).withDeploy(Deploy.local) // disallow remote deployment for typed actors - val mailboxProps = deploy.firstOrElse[MailboxSelector](MailboxSelector.default()) match { + val mailboxProps = props.firstOrElse[MailboxSelector](MailboxSelector.default()) match { case _: DefaultMailboxSelector => dispatcherProps case BoundedMailboxSelector(capacity, _) => // specific support in classic Mailboxes @@ -41,7 +38,7 @@ import akka.dispatch.Mailboxes val localDeploy = mailboxProps.withDeploy(Deploy.local) // disallow remote deployment for typed actors - val tags = deploy.firstOrElse[ActorTags](ActorTagsImpl.empty).tags + val tags = props.firstOrElse[ActorTags](ActorTagsImpl.empty).tags if (tags.isEmpty) localDeploy else localDeploy.withActorTags(tags) } diff --git a/akka-actor-typed/src/main/scala/akka/actor/typed/javadsl/Adapter.scala b/akka-actor-typed/src/main/scala/akka/actor/typed/javadsl/Adapter.scala index 0ddb385516..127f7e8992 100644 --- a/akka-actor-typed/src/main/scala/akka/actor/typed/javadsl/Adapter.scala +++ b/akka-actor-typed/src/main/scala/akka/actor/typed/javadsl/Adapter.scala @@ -11,6 +11,7 @@ import akka.actor.typed.ActorRef import akka.actor.typed.scaladsl.adapter._ import akka.actor.typed.ActorSystem import akka.actor.typed.Scheduler +import akka.actor.typed.SupervisorStrategy import akka.actor.typed.internal.adapter.ActorContextAdapter import akka.japi.Creator @@ -143,7 +144,10 @@ object Adapter { * example of that. */ def props[T](behavior: Creator[Behavior[T]], deploy: Props): akka.actor.Props = - akka.actor.typed.internal.adapter.PropsAdapter(() => behavior.create(), deploy) + akka.actor.typed.internal.adapter.PropsAdapter( + () => Behaviors.supervise(behavior.create()).onFailure(SupervisorStrategy.stop), + deploy, + rethrowTypedFailure = false) /** * Wrap [[akka.actor.typed.Behavior]] in a classic [[akka.actor.Props]], i.e. when diff --git a/akka-actor-typed/src/main/scala/akka/actor/typed/scaladsl/adapter/PropsAdapter.scala b/akka-actor-typed/src/main/scala/akka/actor/typed/scaladsl/adapter/PropsAdapter.scala index 8998714678..482dbc88ed 100644 --- a/akka-actor-typed/src/main/scala/akka/actor/typed/scaladsl/adapter/PropsAdapter.scala +++ b/akka-actor-typed/src/main/scala/akka/actor/typed/scaladsl/adapter/PropsAdapter.scala @@ -6,6 +6,8 @@ package akka.actor.typed.scaladsl.adapter import akka.actor.typed.Behavior import akka.actor.typed.Props +import akka.actor.typed.SupervisorStrategy +import akka.actor.typed.scaladsl.Behaviors /** * Wrap [[akka.actor.typed.Behavior]] in a classic [[akka.actor.Props]], i.e. when @@ -18,5 +20,8 @@ import akka.actor.typed.Props */ object PropsAdapter { def apply[T](behavior: => Behavior[T], deploy: Props = Props.empty): akka.actor.Props = - akka.actor.typed.internal.adapter.PropsAdapter(() => behavior, deploy) + akka.actor.typed.internal.adapter.PropsAdapter( + () => Behaviors.supervise(behavior).onFailure(SupervisorStrategy.stop), + deploy, + rethrowTypedFailure = false) }