Merge pull request #1236 from akka/wip-3114-replace-props-on-terminate-√

#3114 - Replace props on ActorCell termination to avoid leakage if one ...

akka-actor-tests/src/test/scala/akka/actor/LocalActorRefProviderSpec.scala

@@ -57,6 +57,24 @@ class LocalActorRefProviderSpec extends AkkaSpec(LocalActorRefProviderSpec.confi
 
   }
 
+  "A LocalActorRef's ActorCell" must {
+    "not retain its original Props when terminated" in {
+      val GetChild = "GetChild"
+      val a = watch(system.actorOf(Props(new Actor {
+        val child = context.actorOf(Props.empty)
+        def receive = { case `GetChild` ⇒ sender ! child }
+      })))
+      a.tell(GetChild, testActor)
+      val child = expectMsgType[ActorRef]
+      child.asInstanceOf[LocalActorRef].underlying.props must be theSameInstanceAs Props.empty
+      system stop a
+      expectMsgType[Terminated]
+      val childProps = child.asInstanceOf[LocalActorRef].underlying.props
+      childProps must not be theSameInstanceAs(Props.empty)
+      childProps must be theSameInstanceAs ActorCell.terminatedProps
+    }
+  }
+
   "An ActorRefFactory" must {
     implicit val ec = system.dispatcher
     "only create one instance of an actor with a specific address in a concurrent environment" in {

akka-actor/src/main/scala/akka/actor/ActorCell.scala

@@ -304,6 +304,8 @@ private[akka] object ActorCell {
   final val emptyBehaviorStack: List[Actor.Receive] = Nil
 
   final val emptyActorRefSet: Set[ActorRef] = immutable.TreeSet.empty
+
+  final val terminatedProps: Props = Props(() ⇒ throw new IllegalActorStateException("This Actor has been terminated"))
 }
 
 //ACTORCELL IS 64bytes and should stay that way unless very good reason not to (machine sympathy, cache line fit)
@@ -521,36 +523,35 @@ private[akka] class ActorCell(
     case _                               ⇒
   }
 
+  @tailrec private final def lookupAndSetField(clazz: Class[_], instance: AnyRef, name: String, value: Any): Boolean = {
+    if (try {
+      val field = clazz.getDeclaredField(name)
+      field.setAccessible(true)
+      field.set(instance, value)
+      true
+    } catch {
+      case e: NoSuchFieldException ⇒ false
+    }) true
+    else if (clazz.getSuperclass eq null) false
+    else lookupAndSetField(clazz.getSuperclass, instance, name, value)
+  }
+
+  final protected def clearActorCellFields(cell: ActorCell): Unit =
+    if (!lookupAndSetField(cell.getClass, cell, "props", ActorCell.terminatedProps))
+      throw new IllegalArgumentException("ActorCell has no props field")
+
   final protected def clearActorFields(actorInstance: Actor): Unit = {
     setActorFields(actorInstance, context = null, self = system.deadLetters)
     currentMessage = null
     behaviorStack = emptyBehaviorStack
   }
 
-  final protected def setActorFields(actorInstance: Actor, context: ActorContext, self: ActorRef) {
-    @tailrec
-    def lookupAndSetField(clazz: Class[_], actor: Actor, name: String, value: Any): Boolean = {
-      val success = try {
-        val field = clazz.getDeclaredField(name)
-        field.setAccessible(true)
-        field.set(actor, value)
-        true
-      } catch {
-        case e: NoSuchFieldException ⇒ false
-      }
-
-      if (success) true
-      else {
-        val parent: Class[_] = clazz.getSuperclass
-        if (parent eq null) throw new IllegalActorStateException(actorInstance.getClass + " is not an Actor since it have not mixed in the 'Actor' trait")
-        lookupAndSetField(parent, actor, name, value)
-      }
-    }
+  final protected def setActorFields(actorInstance: Actor, context: ActorContext, self: ActorRef): Unit =
     if (actorInstance ne null) {
-      lookupAndSetField(actorInstance.getClass, actorInstance, "context", context)
-      lookupAndSetField(actorInstance.getClass, actorInstance, "self", self)
+      if (!lookupAndSetField(actorInstance.getClass, actorInstance, "context", context)
+        || !lookupAndSetField(actorInstance.getClass, actorInstance, "self", self))
+        throw new IllegalActorStateException(actorInstance.getClass + " is not an Actor since it have not mixed in the 'Actor' trait")
     }
-  }
 
   // logging is not the main purpose, and if it fails there’s nothing we can do
   protected final def publish(e: LogEvent): Unit = try system.eventStream.publish(e) catch { case NonFatal(_) ⇒ }

akka-actor/src/main/scala/akka/actor/dungeon/FaultHandling.scala

@@ -194,9 +194,8 @@ private[akka] trait FaultHandling { this: ActorCell ⇒
      * specific order.
      */
     try if (a ne null) a.postStop()
-    catch handleNonFatalOrInterruptedException { e ⇒
-      publish(Error(e, self.path.toString, clazz(a), e.getMessage))
-    } finally try dispatcher.detach(this)
+    catch handleNonFatalOrInterruptedException { e ⇒ publish(Error(e, self.path.toString, clazz(a), e.getMessage)) }
+    finally try dispatcher.detach(this)
     finally try parent.sendSystemMessage(ChildTerminated(self))
     finally try parent ! NullMessage // read ScalaDoc of NullMessage to see why
     finally try tellWatchersWeDied(a)
@@ -204,7 +203,9 @@ private[akka] trait FaultHandling { this: ActorCell ⇒
     finally {
       if (system.settings.DebugLifecycle)
         publish(Debug(self.path.toString, clazz(a), "stopped"))
+
       clearActorFields(a)
+      clearActorCellFields(this)
       actor = null
     }
   }

akka-actor/src/main/scala/akka/routing/Routing.scala

@@ -33,9 +33,7 @@ private[akka] class RoutedActorRef(_system: ActorSystemImpl, _props: Props, _sup
     throw new ConfigurationException(
       "Configuration for " + this +
         " is invalid - you can not use a 'BalancingDispatcher' as a Router's dispatcher, you can however use it for the routees.")
-  }
-
-  _props.routerConfig.verifyConfig()
+  } else _props.routerConfig.verifyConfig()
 
   override def newCell(old: UnstartedCell): Cell = new RoutedActorCell(system, this, props, supervisor).init(old.uid, sendSupervise = false)