From 58324eb2632e39e2d59f8d78f8f29ee75c08447f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Endre=20S=C3=A1ndor=20Varga?= Date: Fri, 4 Jan 2013 11:25:54 +0100 Subject: [PATCH 1/2] Enforcing ordering of Terminated wrt remote/local #2835 --- .../src/main/scala/akka/actor/Address.scala | 1 + .../scala/akka/actor/dungeon/DeathWatch.scala | 26 ++++++++++++++----- .../akka/actor/dungeon/FaultHandling.scala | 6 ++++- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/akka-actor/src/main/scala/akka/actor/Address.scala b/akka-actor/src/main/scala/akka/actor/Address.scala index d98bbcb208..33c147de7a 100644 --- a/akka-actor/src/main/scala/akka/actor/Address.scala +++ b/akka-actor/src/main/scala/akka/actor/Address.scala @@ -19,6 +19,7 @@ import scala.collection.immutable */ @SerialVersionUID(1L) final case class Address private (protocol: String, system: String, host: Option[String], port: Option[Int]) { + // Please note that local/non-local distinction must be preserved: host.isDefined == !isLocal def this(protocol: String, system: String) = this(protocol, system, None, None) def this(protocol: String, system: String, host: String, port: Int) = this(protocol, system, Option(host), Some(port)) diff --git a/akka-actor/src/main/scala/akka/actor/dungeon/DeathWatch.scala b/akka-actor/src/main/scala/akka/actor/dungeon/DeathWatch.scala index 70f79f1d48..313796ad68 100644 --- a/akka-actor/src/main/scala/akka/actor/dungeon/DeathWatch.scala +++ b/akka-actor/src/main/scala/akka/actor/dungeon/DeathWatch.scala @@ -4,7 +4,7 @@ package akka.actor.dungeon -import akka.actor.{ Terminated, InternalActorRef, ActorRef, ActorCell, Actor, Address, AddressTerminated } +import akka.actor.{ Terminated, InternalActorRef, ActorRef, ActorRefScope, ActorCell, Actor, Address, AddressTerminated } import akka.dispatch.{ Watch, Unwatch } import akka.event.Logging.{ Warning, Error, Debug } import scala.util.control.NonFatal @@ -51,12 +51,24 @@ private[akka] trait DeathWatch { this: ActorCell ⇒ if (!watchedBy.isEmpty) { val terminated = Terminated(self)(existenceConfirmed = true, addressTerminated = false) try { - watchedBy foreach { - watcher ⇒ - try watcher.tell(terminated, self) catch { - case NonFatal(t) ⇒ publish(Error(t, self.path.toString, clazz(actor), "deathwatch")) - } - } + def sendTerminated(ifLocal: Boolean)(watcher: ActorRef): Unit = + if (watcher.asInstanceOf[ActorRefScope].isLocal == ifLocal) watcher.tell(terminated, self) + + /* + * It is important to notify the remote watchers first, otherwise RemoteDaemon might shut down, causing + * the remoting to shut down as well. At this point Terminated messages to remote watchers are no longer + * deliverable. + * + * The problematic case is: + * 1. Terminated is sent to RemoteDaemon + * 1a. RemoteDaemon is fast enough to notify the terminator actor in RemoteActorRefProvider + * 1b. The terminator is fast enough to enqueue the shutdown command in the remoting + * 2. Only at this point is the Terminated (to be sent remotely) enqueued in the mailbox of remoting + * + * If the remote watchers are notified first, then the mailbox of the Remoting will guarantee the correct order. + */ + watchedBy foreach sendTerminated(ifLocal = false) + watchedBy foreach sendTerminated(ifLocal = true) } finally watchedBy = ActorCell.emptyActorRefSet } } diff --git a/akka-actor/src/main/scala/akka/actor/dungeon/FaultHandling.scala b/akka-actor/src/main/scala/akka/actor/dungeon/FaultHandling.scala index ac4f5b5c36..c2ff511809 100644 --- a/akka-actor/src/main/scala/akka/actor/dungeon/FaultHandling.scala +++ b/akka-actor/src/main/scala/akka/actor/dungeon/FaultHandling.scala @@ -190,7 +190,11 @@ private[akka] trait FaultHandling { this: ActorCell ⇒ private def finishTerminate() { val a = actor - // The following order is crucial for things to work properly. Only chnage this if you're very confident and lucky. + /* The following order is crucial for things to work properly. Only change this if you're very confident and lucky. + * + * Please note that if a parent is also a watcher then ChildTerminated and Terminated must be processed in this + * specific order. + */ try if (a ne null) a.postStop() finally try dispatcher.detach(this) finally try parent.sendSystemMessage(ChildTerminated(self)) From dc5f835f1f72cf3cd2a31317864cf891904669ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Endre=20S=C3=A1ndor=20Varga?= Date: Fri, 4 Jan 2013 14:24:39 +0100 Subject: [PATCH 2/2] Added scope query methods to Address. --- .../src/main/scala/akka/actor/Address.scala | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/akka-actor/src/main/scala/akka/actor/Address.scala b/akka-actor/src/main/scala/akka/actor/Address.scala index 33c147de7a..f02dcfb567 100644 --- a/akka-actor/src/main/scala/akka/actor/Address.scala +++ b/akka-actor/src/main/scala/akka/actor/Address.scala @@ -19,11 +19,27 @@ import scala.collection.immutable */ @SerialVersionUID(1L) final case class Address private (protocol: String, system: String, host: Option[String], port: Option[Int]) { - // Please note that local/non-local distinction must be preserved: host.isDefined == !isLocal + // Please note that local/non-local distinction must be preserved: + // host.isDefined == hasGlobalScope + // host.isEmpty == hasLocalScope + // hasLocalScope == !hasGlobalScope def this(protocol: String, system: String) = this(protocol, system, None, None) def this(protocol: String, system: String, host: String, port: Int) = this(protocol, system, Option(host), Some(port)) + /** + * Returns true if this Address is only defined locally. It is not safe to send locally scoped addresses to remote + * hosts. See also [[akka.actor.Address#hasGlobalScope]]. + */ + def hasLocalScope: Boolean = host.isEmpty + + /** + * Returns true if this Address is usable globally. Unlike locally defined addresses ([[akka.actor.Address#hasLocalScope]]) + * addresses of global scope are safe to sent to other hosts, as they globally and uniquely identify an addressable + * entity. + */ + def hasGlobalScope: Boolean = host.isDefined + /** * Returns the canonical String representation of this Address formatted as: *