incorporate Viktor’s review

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

@@ -367,7 +367,7 @@ class TypedActorSpec extends AkkaSpec(TypedActorSpec.config)
 
     "be able to serialize and deserialize invocations" in {
       import java.io._
-      JavaSerializer.currentSystem.withValue(system.asInstanceOf[ActorSystemImpl]) {
+      JavaSerializer.currentSystem.withValue(system.asInstanceOf[ExtendedActorSystem]) {
         val m = TypedActor.MethodCall(classOf[Foo].getDeclaredMethod("pigdog"), Array[AnyRef]())
         val baos = new ByteArrayOutputStream(8192 * 4)
         val out = new ObjectOutputStream(baos)
@@ -386,7 +386,7 @@ class TypedActorSpec extends AkkaSpec(TypedActorSpec.config)
     "be able to serialize and deserialize invocations' parameters" in {
       import java.io._
       val someFoo: Foo = new Bar
-      JavaSerializer.currentSystem.withValue(system.asInstanceOf[ActorSystemImpl]) {
+      JavaSerializer.currentSystem.withValue(system.asInstanceOf[ExtendedActorSystem]) {
         val m = TypedActor.MethodCall(classOf[Foo].getDeclaredMethod("testMethodCallSerialization", Array[Class[_]](classOf[Foo], classOf[String], classOf[Int]): _*), Array[AnyRef](someFoo, null, 1.asInstanceOf[AnyRef]))
         val baos = new ByteArrayOutputStream(8192 * 4)
         val out = new ObjectOutputStream(baos)

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

@@ -333,7 +333,7 @@ abstract class ExtendedActorSystem extends ActorSystem {
   def propertyMaster: PropertyMaster
 }
 
-class ActorSystemImpl(val name: String, applicationConfig: Config) extends ExtendedActorSystem {
+class ActorSystemImpl protected[akka] (val name: String, applicationConfig: Config) extends ExtendedActorSystem {
 
   if (!name.matches("""^\w+$"""))
     throw new IllegalArgumentException("invalid ActorSystem name [" + name + "], must contain only word characters (i.e. [a-zA-Z_0-9])")
@@ -360,14 +360,10 @@ class ActorSystemImpl(val name: String, applicationConfig: Config) extends Exten
    * This is an extension point: by overriding this method, subclasses can
    * control all reflection activities of an actor system.
    */
-  protected def createPropertyMaster(): PropertyMaster = new DefaultPropertyMaster(findClassLoader)
+  protected def createPropertyMaster(): PropertyMaster = new ReflectivePropertyMaster(findClassLoader)
 
-  protected def findClassLoader: ClassLoader =
-    Option(Thread.currentThread.getContextClassLoader) orElse
-      (Reflect.getCallerClass map findCaller) getOrElse
-      getClass.getClassLoader
-
-  private def findCaller(get: Int ⇒ Class[_]): ClassLoader = {
+  protected def findClassLoader: ClassLoader = {
+    def findCaller(get: Int ⇒ Class[_]): ClassLoader = {
       val frames = Iterator.from(2).map(get)
       frames dropWhile { c ⇒
         c != null &&
@@ -381,6 +377,11 @@ class ActorSystemImpl(val name: String, applicationConfig: Config) extends Exten
       }
     }
 
+    Option(Thread.currentThread.getContextClassLoader) orElse
+      (Reflect.getCallerClass map findCaller) getOrElse
+      getClass.getClassLoader
+  }
+
   private val _pm: PropertyMaster = createPropertyMaster()
   def propertyMaster: PropertyMaster = _pm
 

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

@@ -8,19 +8,38 @@ import java.lang.reflect.InvocationTargetException
 
 /**
  * The property master is responsible for acquiring all props needed for a
- * performance; in Akka this is the class which is used for reflectively
- * loading all configurable parts of an actor system.
+ * performance; in Akka this is the class which is used for
+ * loading all configurable parts of an actor system (the
+ * [[akka.actor.ReflectivePropertyMaster]] is the default implementation).
+ *
+ * This is an internal facility and users are not expected to encounter it
+ * unless they are extending Akka in ways which go beyond simple Extensions.
  */
-trait PropertyMaster {
+private[akka] trait PropertyMaster {
 
+  /**
+   * Obtain a `Class[_]` object loaded with the right class loader (i.e. the one
+   * returned by `classLoader`).
+   */
   def getClassFor[T: ClassManifest](fqcn: String): Either[Throwable, Class[_ <: T]]
 
+  /**
+   * Obtain an object conforming to the type T, which is expected to be
+   * instantiated from a class designated by the fully-qualified class name
+   * given, where the constructor is selected and invoked according to the
+   * `args` argument. The exact usage of args depends on which type is requested,
+   * see the relevant requesting code for details.
+   */
   def getInstanceFor[T: ClassManifest](fqcn: String, args: Seq[(Class[_], AnyRef)]): Either[Throwable, T]
 
+  /**
+   * Obtain the Scala “object” instance for the given fully-qualified class name, if there is one.
+   */
   def getObjectFor[T: ClassManifest](fqcn: String): Either[Throwable, T]
 
   /**
-   * This is needed e.g. by the JavaSerializer to build the ObjectInputStream.
+   * This is the class loader to be used in those special cases where the
+   * other factory method are not applicable (e.g. when constructing a ClassLoaderBinaryInputStream).
    */
   def classLoader: ClassLoader
 
@@ -28,6 +47,14 @@ trait PropertyMaster {
 
 object PropertyMaster {
 
+  /**
+   * Convenience method which given a `Class[_]` object and a constructor description
+   * will create a new instance of that class.
+   *
+   * {{{
+   * val obj = PropertyMaster.getInstanceFor(clazz, Seq(classOf[Config] -> config, classOf[String] -> name))
+   * }}}
+   */
   def getInstanceFor[T: ClassManifest](clazz: Class[_], args: Seq[(Class[_], AnyRef)]): Either[Throwable, T] = {
     val types = args.map(_._1).toArray
     val values = args.map(_._2).toArray
@@ -58,7 +85,14 @@ object PropertyMaster {
 
 }
 
-class DefaultPropertyMaster(val classLoader: ClassLoader) extends PropertyMaster {
+/**
+ * This is the default [[akka.actor.PropertyMaster]] implementation used by [[akka.actor.ActorSystemImpl]]
+ * unless overridden. It uses reflection to turn fully-qualified class names into `Class[_]` objects
+ * and creates instances from there using `getDeclaredConstructor()` and invoking that. The class loader
+ * to be used for all this is determined by the [[akka.actor.ActorSystemImpl]]’s `findClassLoader` method
+ * by default.
+ */
+class ReflectivePropertyMaster(val classLoader: ClassLoader) extends PropertyMaster {
 
   import PropertyMaster.withErrorHandling
 

akka-actor/src/main/scala/akka/serialization/Format.scala

@@ -1,104 +0,0 @@
-package akka.serialization
-
-/**
- * Copyright (C) 2009-2012 Typesafe Inc. <http://www.typesafe.com>
- */
-
-import akka.actor.Actor
-
-/**
- * trait Serializer extends scala.Serializable {
- * @volatile
- * var classLoader: Option[ClassLoader] = None
- * def deepClone(obj: AnyRef): AnyRef = fromBinary(toBinary(obj), Some(obj.getClass))
- *
- * def toBinary(obj: AnyRef): Array[Byte]
- * def fromBinary(bytes: Array[Byte], clazz: Option[Class[_]]): AnyRef
- * }
- */
-
-/**
- *
- * object Format {
- * implicit object Default extends Serializer {
- * import java.io.{ ObjectOutputStream, ByteArrayOutputStream, ObjectInputStream, ByteArrayInputStream }
- * //import org.apache.commons.io.input.ClassLoaderObjectInputStream
- *
- * def toBinary(obj: AnyRef): Array[Byte] = {
- * val bos = new ByteArrayOutputStream
- * val out = new ObjectOutputStream(bos)
- * out.writeObject(obj)
- * out.close()
- * bos.toByteArray
- * }
- *
- * def fromBinary(bytes: Array[Byte], clazz: Option[Class[_]], classLoader: Option[ClassLoader] = None): AnyRef = {
- * val in =
- * //if (classLoader.isDefined) new ClassLoaderObjectInputStream(classLoader.get, new ByteArrayInputStream(bytes)) else
- * new ObjectInputStream(new ByteArrayInputStream(bytes))
- * val obj = in.readObject
- * in.close()
- * obj
- * }
- *
- * def identifier: Byte = 111 //Pick a number and hope no one has chosen the same :-) 0 - 16 is reserved for Akka internals
- *
- * }
- *
- * val defaultSerializerName = Default.getClass.getName
- * }
- */
-
-trait FromBinary[T <: Actor] {
-  def fromBinary(bytes: Array[Byte], act: T): T
-}
-
-trait ToBinary[T <: Actor] {
-  def toBinary(t: T): Array[Byte]
-}
-
-/**
- * Type class definition for Actor Serialization.
- * Client needs to implement Format[] for the respective actor.
- */
-// FIXME RK: should this go? It’s not used anywhere, looks like cluster residue.
-trait Format[T <: Actor] extends FromBinary[T] with ToBinary[T]
-
-/**
- * A default implementation for a stateless actor
- *
- * Create a Format object with the client actor as the implementation of the type class
- *
- * <pre>
- * object BinaryFormatMyStatelessActor  {
- *   implicit object MyStatelessActorFormat extends StatelessActorFormat[MyStatelessActor]
- * }
- * </pre>
- */
-trait StatelessActorFormat[T <: Actor] extends Format[T] with scala.Serializable {
-  def fromBinary(bytes: Array[Byte], act: T) = act
-
-  def toBinary(ac: T) = Array.empty[Byte]
-}
-
-/**
- * A default implementation of the type class for a Format that specifies a serializer
- *
- * Create a Format object with the client actor as the implementation of the type class and
- * a serializer object
- *
- * <pre>
- * object BinaryFormatMyJavaSerializableActor  {
- *   implicit object MyJavaSerializableActorFormat extends SerializerBasedActorFormat[MyJavaSerializableActor]  {
- *     val serializer = Serializers.Java
- *   }
- * }
- * </pre>
- */
-trait SerializerBasedActorFormat[T <: Actor] extends Format[T] with scala.Serializable {
-  val serializer: Serializer
-
-  def fromBinary(bytes: Array[Byte], act: T): T = serializer.fromBinary(bytes, Some(act.getClass)).asInstanceOf[T]
-
-  def toBinary(ac: T): Array[Byte] = serializer.toBinary(ac)
-}

akka-actor/src/main/scala/akka/serialization/Serialization.scala

@@ -11,6 +11,7 @@ import akka.config.ConfigurationException
 import akka.actor.{ Extension, ExtendedActorSystem, Address }
 import java.util.concurrent.ConcurrentHashMap
 import akka.event.Logging
+import akka.util.NonFatal
 
 case class NoSerializerFoundException(m: String) extends AkkaException(m)
 
@@ -59,9 +60,9 @@ class Serialization(val system: ExtendedActorSystem) extends Extension {
    * Serializes the given AnyRef/java.lang.Object according to the Serialization configuration
    * to either an Array of Bytes or an Exception if one was thrown.
    */
-  def serialize(o: AnyRef): Either[Exception, Array[Byte]] =
+  def serialize(o: AnyRef): Either[Throwable, Array[Byte]] =
     try Right(findSerializerFor(o).toBinary(o))
-    catch { case e: Exception ⇒ Left(e) }
+    catch { case NonFatal(e) ⇒ Left(e) }
 
   /**
    * Deserializes the given array of bytes using the specified serializer id,
@@ -70,18 +71,18 @@ class Serialization(val system: ExtendedActorSystem) extends Extension {
    */
   def deserialize(bytes: Array[Byte],
                   serializerId: Int,
-                  clazz: Option[Class[_]]): Either[Exception, AnyRef] =
+                  clazz: Option[Class[_]]): Either[Throwable, AnyRef] =
     try Right(serializerByIdentity(serializerId).fromBinary(bytes, clazz))
-    catch { case e: Exception ⇒ Left(e) }
+    catch { case NonFatal(e) ⇒ Left(e) }
 
   /**
    * Deserializes the given array of bytes using the specified type to look up what Serializer should be used.
    * You can specify an optional ClassLoader to load the object into.
    * Returns either the resulting object or an Exception if one was thrown.
    */
-  def deserialize(bytes: Array[Byte], clazz: Class[_]): Either[Exception, AnyRef] =
+  def deserialize(bytes: Array[Byte], clazz: Class[_]): Either[Throwable, AnyRef] =
     try Right(serializerFor(clazz).fromBinary(bytes, Some(clazz)))
-    catch { case e: Exception ⇒ Left(e) }
+    catch { case NonFatal(e) ⇒ Left(e) }
 
   /**
    * Returns the Serializer configured for the given object, returns the NullSerializer if it's null,
@@ -126,7 +127,8 @@ class Serialization(val system: ExtendedActorSystem) extends Extension {
     }
 
   /**
-   * Tries to load the specified Serializer by the FQN
+   * Tries to load the specified Serializer by the fully-qualified name; the actual
+   * loading is performed by the system’s [[akka.actor.PropertyMaster]].
    */
   def serializerOf(serializerFQN: String): Either[Throwable, Serializer] = {
     val pm = system.propertyMaster

akka-actor/src/main/scala/akka/serialization/Serializer.scala

@@ -25,7 +25,9 @@ import scala.util.DynamicVariable
  * load classes using reflection.</li>
  * </ul>
  *
- * <b>Be sure to always use the PropertyManager for loading classes!</b>
+ * <b>Be sure to always use the PropertyManager for loading classes!</b> This is necessary to
+ * avoid strange match errors and inequalities which arise from different class loaders loading
+ * the same class.
  */
 trait Serializer extends scala.Serializable {
 

akka-actor/src/main/scala/akka/util/Reflect.scala

@@ -3,13 +3,25 @@
  */
 package akka.util
 
+/**
+ * Collection of internal reflection utilities which may or may not be
+ * available (most services specific to HotSpot, but fails gracefully).
+ */
 object Reflect {
 
+  /**
+   * This optionally holds a function which looks N levels above itself
+   * on the call stack and returns the `Class[_]` object for the code
+   * executing in that stack frame. Implemented using
+   * `sun.reflect.Reflection.getCallerClass` if available, None otherwise.
+   *
+   * Hint: when comparing to Thread.currentThread.getStackTrace, add two levels.
+   */
   val getCallerClass: Option[Int ⇒ Class[_]] = {
     try {
       val c = Class.forName("sun.reflect.Reflection");
       val m = c.getMethod("getCallerClass", Array(classOf[Int]): _*)
-      Some((i: Int) ⇒ m.invoke(null, Array[AnyRef](i.asInstanceOf[Integer]): _*).asInstanceOf[Class[_]])
+      Some((i: Int) ⇒ m.invoke(null, Array[AnyRef](i.asInstanceOf[java.lang.Integer]): _*).asInstanceOf[Class[_]])
     } catch {
       case NonFatal(e) ⇒ None
     }

akka-docs/java/serialization.rst

@@ -111,3 +111,14 @@ which is done by extending ``akka.serialization.JSerializer``, like this:
 
 Then you only need to fill in the blanks, bind it to a name in your :ref:`configuration` and then
 list which classes that should be serialized using it.
+
+A Word About Java Serialization
+===============================
+
+When using Java serialization without employing the :class:`JavaSerializer` for
+the task, you must make sure to supply a valid :class:`ExtendedActorSystem` in
+the dynamic variable ``JavaSerializer.currentSystem``. This is used when
+reading in the representation of an :class:`ActorRef` for turning the string
+representation into a real reference. :class:`DynamicVariable` is a
+thread-local variable, so be sure to have it set while deserializing anything
+which might contain actor references.

akka-docs/scala/serialization.rst

@@ -109,3 +109,14 @@ First you need to create a class definition of your ``Serializer`` like so:
 
 Then you only need to fill in the blanks, bind it to a name in your :ref:`configuration` and then
 list which classes that should be serialized using it.
+
+A Word About Java Serialization
+===============================
+
+When using Java serialization without employing the :class:`JavaSerializer` for
+the task, you must make sure to supply a valid :class:`ExtendedActorSystem` in
+the dynamic variable ``JavaSerializer.currentSystem``. This is used when
+reading in the representation of an :class:`ActorRef` for turning the string
+representation into a real reference. :class:`DynamicVariable` is a
+thread-local variable, so be sure to have it set while deserializing anything
+which might contain actor references.