From 5478fe094c97fc6c72936aee8d4066cc35ef98d9 Mon Sep 17 00:00:00 2001
From: Viktor Klang <viktor.klang@gmail.com>
Date: Wed, 14 Nov 2012 14:34:31 +0100
Subject: [PATCH] Simplifying the code inside Children and RepointableActorRef

---
 .../akka/actor/RepointableActorRef.scala      | 24 ++++++-----
 .../scala/akka/actor/dungeon/Children.scala   | 43 ++++++++++---------
 2 files changed, 35 insertions(+), 32 deletions(-)

diff --git a/akka-actor/src/main/scala/akka/actor/RepointableActorRef.scala b/akka-actor/src/main/scala/akka/actor/RepointableActorRef.scala
index 1f1c9cae0a..87c02455bc 100644
--- a/akka-actor/src/main/scala/akka/actor/RepointableActorRef.scala
+++ b/akka-actor/src/main/scala/akka/actor/RepointableActorRef.scala
@@ -52,12 +52,15 @@ private[akka] class RepointableActorRef(
    *
    * This is protected so that others can have different initialization.
    */
-  def initialize(): this.type = {
-    val uid = ThreadLocalRandom.current.nextInt()
-    swapCell(new UnstartedCell(system, this, props, supervisor, uid))
-    supervisor.sendSystemMessage(Supervise(this, uid))
-    this
-  }
+  def initialize(): this.type =
+    underlying match {
+      case null ⇒
+        val uid = ThreadLocalRandom.current.nextInt()
+        swapCell(new UnstartedCell(system, this, props, supervisor, uid))
+        supervisor.sendSystemMessage(Supervise(this, uid))
+        this
+      case other ⇒ this
+    }
 
   /**
    * This method is supposed to be called by the supervisor in handleSupervise()
@@ -65,13 +68,12 @@ private[akka] class RepointableActorRef(
    * modification of the `underlying` field, though it is safe to send messages
    * at any time.
    */
-  def activate(): this.type = {
+  def activate(): this.type =
     underlying match {
-      case u: UnstartedCell ⇒ u.replaceWith(newCell(u))
-      case _                ⇒ // this happens routinely for things which were created async=false
+      case u: UnstartedCell ⇒ u.replaceWith(newCell(u)); this
+      case null             ⇒ throw new IllegalStateException("underlying cell is null")
+      case _                ⇒ this // this happens routinely for things which were created async=false
     }
-    this
-  }
 
   /**
    * This is called by activate() to obtain the cell which is to replace the
diff --git a/akka-actor/src/main/scala/akka/actor/dungeon/Children.scala b/akka-actor/src/main/scala/akka/actor/dungeon/Children.scala
index 74e06f3034..a9c1383374 100644
--- a/akka-actor/src/main/scala/akka/actor/dungeon/Children.scala
+++ b/akka-actor/src/main/scala/akka/actor/dungeon/Children.scala
@@ -52,19 +52,24 @@ private[akka] trait Children { this: ActorCell ⇒
   }
 
   final def stop(actor: ActorRef): Unit = {
-    val started = actor match {
-      case r: RepointableRef ⇒ r.isStarted
-      case _                 ⇒ true
+    if (childrenRefs.getByRef(actor).isDefined) {
+      @tailrec def shallDie(ref: ActorRef): Boolean = {
+        val c = childrenRefs
+        swapChildrenRefs(c, c.shallDie(ref)) || shallDie(ref)
+      }
+
+      if (actor match {
+        case r: RepointableRef ⇒ r.isStarted
+        case _                 ⇒ true
+      }) shallDie(actor)
     }
-    if (childrenRefs.getByRef(actor).isDefined && started) shallDie(actor)
     actor.asInstanceOf[InternalActorRef].stop()
   }
 
   /*
    * low level CAS helpers
    */
-
-  @inline private def swapChildrenRefs(oldChildren: ChildrenContainer, newChildren: ChildrenContainer): Boolean =
+  @inline private final def swapChildrenRefs(oldChildren: ChildrenContainer, newChildren: ChildrenContainer): Boolean =
     Unsafe.instance.compareAndSwapObject(this, AbstractActorCell.childrenOffset, oldChildren, newChildren)
 
   @tailrec final def reserveChild(name: String): Boolean = {
@@ -89,18 +94,6 @@ private[akka] trait Children { this: ActorCell ⇒
     }
   }
 
-  @tailrec final protected def shallDie(ref: ActorRef): Boolean = {
-    val c = childrenRefs
-    swapChildrenRefs(c, c.shallDie(ref)) || shallDie(ref)
-  }
-
-  @tailrec final private def removeChild(ref: ActorRef): ChildrenContainer = {
-    val c = childrenRefs
-    val n = c.remove(ref)
-    if (swapChildrenRefs(c, n)) n
-    else removeChild(ref)
-  }
-
   @tailrec final protected def setChildrenTerminationReason(reason: ChildrenContainer.SuspendReason): Boolean = {
     childrenRefs match {
       case c: ChildrenContainer.TerminatingChildrenContainer ⇒
@@ -143,10 +136,18 @@ private[akka] trait Children { this: ActorCell ⇒
   protected def getAllChildStats: immutable.Iterable[ChildRestartStats] = childrenRefs.stats
 
   protected def removeChildAndGetStateChange(child: ActorRef): Option[SuspendReason] = {
-    childrenRefs match {
+    @tailrec def removeChild(ref: ActorRef): ChildrenContainer = {
+      val c = childrenRefs
+      val n = c.remove(ref)
+      if (swapChildrenRefs(c, n)) n else removeChild(ref)
+    }
+
+    childrenRefs match { // The match must be performed BEFORE the removeChild
       case TerminatingChildrenContainer(_, _, reason) ⇒
-        val newContainer = removeChild(child)
-        if (!newContainer.isInstanceOf[TerminatingChildrenContainer]) Some(reason) else None
+        removeChild(child) match {
+          case c: TerminatingChildrenContainer ⇒ None
+          case _                               ⇒ Some(reason)
+        }
       case _ ⇒
         removeChild(child)
         None