From 716893263a3490bb97e761a82859c82f92b80543 Mon Sep 17 00:00:00 2001
From: Patrik Nordwall <patrik.nordwall@gmail.com>
Date: Fri, 24 May 2013 14:43:01 +0200
Subject: [PATCH] ClusterSingletonManagerSpec must not use 'node' from other
 thread, see #3382

* Thank you @rkuhn for finding the reason for the problem
* Sprinkled some warnings
---
 akka-contrib/docs/cluster-singleton.rst             |  6 ++++++
 .../contrib/pattern/ClusterSingletonManager.scala   | 13 ++++++++++++-
 .../pattern/ClusterSingletonManagerSpec.scala       |  5 ++++-
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/akka-contrib/docs/cluster-singleton.rst b/akka-contrib/docs/cluster-singleton.rst
index 1bed5a29e1..f1a81b93f0 100644
--- a/akka-contrib/docs/cluster-singleton.rst
+++ b/akka-contrib/docs/cluster-singleton.rst
@@ -71,6 +71,12 @@ In Java:
 
 .. includecode:: @contribSrc@/src/test/java/akka/contrib/pattern/ClusterSingletonManagerTest.java#create-singleton-manager
 
+.. note::
+
+  The ``singletonProps``/``singletonPropsFactory`` is invoked when creating
+  the singleton actor and it must not use members that are not thread safe, e.g.
+  mutable state in enclosing actor.
+
 Here we use an application specific ``terminationMessage`` to be able to close the
 resources before actually stopping the singleton actor. Note that ``PoisonPill`` is a
 perfectly fine ``terminationMessage`` if you only need to stop the actor.
diff --git a/akka-contrib/src/main/scala/akka/contrib/pattern/ClusterSingletonManager.scala b/akka-contrib/src/main/scala/akka/contrib/pattern/ClusterSingletonManager.scala
index 2a2d7b95b1..aeccb17d96 100644
--- a/akka-contrib/src/main/scala/akka/contrib/pattern/ClusterSingletonManager.scala
+++ b/akka-contrib/src/main/scala/akka/contrib/pattern/ClusterSingletonManager.scala
@@ -25,6 +25,9 @@ object ClusterSingletonManager {
 
   /**
    * Scala API: Factory method for `ClusterSingletonManager` [[akka.actor.Props]].
+   * Note that the `singletonProps` function is applied when creating
+   * the singleton actor and it must not use members that are not thread safe, e.g.
+   * mutable state in enclosing actor.
    */
   def props(
     singletonProps: Option[Any] ⇒ Props,
@@ -40,6 +43,9 @@ object ClusterSingletonManager {
 
   /**
    * Java API: Factory method for `ClusterSingletonManager` [[akka.actor.Props]].
+   * Note that the `singletonPropsFactory` is invoked when creating
+   * the singleton actor and it must not use members that are not thread safe, e.g.
+   * mutable state in enclosing actor.
    */
   def props(
     singletonName: String,
@@ -56,6 +62,9 @@ object ClusterSingletonManager {
   /**
    * Java API: Factory method for `ClusterSingletonManager` [[akka.actor.Props]]
    * with default values.
+   * Note that the `singletonPropsFactory` is invoked when creating
+   * the singleton actor and it must not use members that are not thread safe, e.g.
+   * mutable state in enclosing actor.
    */
   def defaultProps(
     singletonName: String,
@@ -327,7 +336,9 @@ class ClusterSingletonManagerIsStuck(message: String) extends AkkaException(mess
  *    might be None when no hand-over took place, or when the there
  *    is no need for sending data to the new singleton. The `handOverData`
  *    is typically passed as parameter to the constructor of the
- *    singleton actor.
+ *    singleton actor. Note that the `singletonProps` function is applied when creating
+ *    the singleton actor and it must not use members that are not thread safe, e.g.
+ *    mutable state in enclosing actor.
  *
  * '''''singletonName''''' The actor name of the child singleton actor.
  *
diff --git a/akka-contrib/src/multi-jvm/scala/akka/contrib/pattern/ClusterSingletonManagerSpec.scala b/akka-contrib/src/multi-jvm/scala/akka/contrib/pattern/ClusterSingletonManagerSpec.scala
index c2c8fdd993..1961d1cf1a 100644
--- a/akka-contrib/src/multi-jvm/scala/akka/contrib/pattern/ClusterSingletonManagerSpec.scala
+++ b/akka-contrib/src/multi-jvm/scala/akka/contrib/pattern/ClusterSingletonManagerSpec.scala
@@ -195,8 +195,11 @@ class ClusterSingletonManagerSpec extends MultiNodeSpec(ClusterSingletonManagerS
 
   val identifyProbe = TestProbe()
 
+  val controllerRootActorPath = node(controller)
+
   def queue: ActorRef = {
-    system.actorSelection(node(controller) / "user" / "queue").tell(Identify("queue"), identifyProbe.ref)
+    // this is used from inside actor construction, i.e. other thread, and must therefore not call `node(controller`
+    system.actorSelection(controllerRootActorPath / "user" / "queue").tell(Identify("queue"), identifyProbe.ref)
     identifyProbe.expectMsgType[ActorIdentity].ref.get
   }