From 5102d14168d62c7dd05bed281f0a5c0d672c621c Mon Sep 17 00:00:00 2001 From: Josep Prat Date: Sun, 5 Mar 2017 14:39:37 +0100 Subject: [PATCH] Move inlined example to its own class (#22453) Issue: #22453 Mode inlined example for Shared Mutable State to its own class Add additional wrong case when the message is mutable Includes auto reformated code --- .../scala/akka/cluster/sharding/Shard.scala | 4 +- akka-docs/rst/general/jmm.rst | 30 +------ .../actor/SharedMutableStateDocSpec.scala | 80 +++++++++++++++++++ .../DaemonMsgCreateSerializerSpec.scala | 8 +- 4 files changed, 87 insertions(+), 35 deletions(-) create mode 100644 akka-docs/rst/scala/code/docs/actor/SharedMutableStateDocSpec.scala diff --git a/akka-cluster-sharding/src/main/scala/akka/cluster/sharding/Shard.scala b/akka-cluster-sharding/src/main/scala/akka/cluster/sharding/Shard.scala index 14db4c577b..ce14ca82d3 100644 --- a/akka-cluster-sharding/src/main/scala/akka/cluster/sharding/Shard.scala +++ b/akka-cluster-sharding/src/main/scala/akka/cluster/sharding/Shard.scala @@ -228,7 +228,7 @@ private[akka] class Shard( def passivate(entity: ActorRef, stopMessage: Any): Unit = { idByRef.get(entity) match { - case Some(id) => if (!messageBuffers.contains(id)) { + case Some(id) ⇒ if (!messageBuffers.contains(id)) { log.debug("Passivating started on entity {}", id) passivating = passivating + entity @@ -237,7 +237,7 @@ private[akka] class Shard( } else { log.debug("Passivation already in progress for {}. Not sending stopMessage back to entity.", entity) } - case None => log.debug("Unknown entity {}. Not sending stopMessage back to entity.", entity) + case None ⇒ log.debug("Unknown entity {}. Not sending stopMessage back to entity.", entity) } } diff --git a/akka-docs/rst/general/jmm.rst b/akka-docs/rst/general/jmm.rst index 7d0c507361..774f8e28a6 100644 --- a/akka-docs/rst/general/jmm.rst +++ b/akka-docs/rst/general/jmm.rst @@ -72,34 +72,6 @@ Since Akka runs on the JVM there are still some rules to be followed. * Closing over internal Actor state and exposing it to other threads -.. code-block:: scala - - class MyActor extends Actor { - var state = ... - def receive = { - case _ => - //Wrongs - - // Very bad, shared mutable state, - // will break your application in weird ways - Future { state = NewState } - anotherActor ? message onSuccess { r => state = r } - - // Very bad, "sender" changes for every message, - // shared mutable state bug - Future { expensiveCalculation(sender()) } - - //Rights - - // Completely safe, "self" is OK to close over - // and it's an ActorRef, which is thread-safe - Future { expensiveCalculation() } onComplete { f => self ! f.value.get } - - // Completely safe, we close over a fixed value - // and it's an ActorRef, which is thread-safe - val currentSender = sender() - Future { expensiveCalculation(currentSender) } - } - } +.. includecode:: ../scala/code/docs/actor/SharedMutableStateDocSpec.scala#mutable-state * Messages **should** be immutable, this is to avoid the shared mutable state trap. diff --git a/akka-docs/rst/scala/code/docs/actor/SharedMutableStateDocSpec.scala b/akka-docs/rst/scala/code/docs/actor/SharedMutableStateDocSpec.scala new file mode 100644 index 0000000000..6af4d5bc95 --- /dev/null +++ b/akka-docs/rst/scala/code/docs/actor/SharedMutableStateDocSpec.scala @@ -0,0 +1,80 @@ +/** + * Copyright (C) 2009-2017 Lightbend Inc. + */ +package docs.actor + +class SharedMutableStateDocSpec { + + //#mutable-state + import akka.actor.{ Actor, ActorRef } + import akka.pattern.ask + import akka.util.Timeout + import scala.concurrent.Future + import scala.concurrent.duration._ + import scala.language.postfixOps + import scala.collection.mutable + + case class Message(msg: String) + + class EchoActor extends Actor { + def receive = { + case msg => sender() ! msg + } + } + + class CleanUpActor extends Actor { + def receive = { + case set: mutable.Set[_] => set.clear() + } + } + + class MyActor(echoActor: ActorRef, cleanUpActor: ActorRef) extends Actor { + var state = "" + val mySet = mutable.Set[String]() + + def expensiveCalculation(actorRef: ActorRef): String = { + // this is a very costly operation + "Meaning of live is 42" + } + + def expensiveCalculation(): String = { + // this is a very costly operation + "Meaning of live is 42" + } + + def receive = { + case _ => + + //Wrong ways + implicit val ec = context.dispatcher + implicit val timeout = Timeout(5 seconds) // needed for `?` below + + // Very bad, shared mutable state, + // will break your application in weird ways + Future { state = "This will race" } + ((echoActor ? Message("With this other one")).mapTo[Message]) + .foreach { received => state = received.msg } + + // Very bad, shared mutable object, + // the other actor cand mutate your own state, + // or worse, you might get weird race conditions + cleanUpActor ! mySet + + // Very bad, "sender" changes for every message, + // shared mutable state bug + Future { expensiveCalculation(sender()) } + + //Right ways + + // Completely safe, "self" is OK to close over + // and it's an ActorRef, which is thread-safe + Future { expensiveCalculation() } foreach { self ! _ } + + // Completely safe, we close over a fixed value + // and it's an ActorRef, which is thread-safe + val currentSender = sender() + Future { expensiveCalculation(currentSender) } + } + } + //#mutable-state +} diff --git a/akka-remote/src/test/scala/akka/remote/serialization/DaemonMsgCreateSerializerSpec.scala b/akka-remote/src/test/scala/akka/remote/serialization/DaemonMsgCreateSerializerSpec.scala index c867b44a3e..058c591aca 100644 --- a/akka-remote/src/test/scala/akka/remote/serialization/DaemonMsgCreateSerializerSpec.scala +++ b/akka-remote/src/test/scala/akka/remote/serialization/DaemonMsgCreateSerializerSpec.scala @@ -4,11 +4,11 @@ package akka.remote.serialization -import akka.actor.{Actor, ActorRef, ActorSystem, Address, Deploy, ExtendedActorSystem, OneForOneStrategy, Props, SupervisorStrategy} -import akka.remote.{DaemonMsgCreate, RemoteScope} -import akka.routing.{FromConfig, RoundRobinPool} +import akka.actor.{ Actor, ActorRef, ActorSystem, Address, Deploy, ExtendedActorSystem, OneForOneStrategy, Props, SupervisorStrategy } +import akka.remote.{ DaemonMsgCreate, RemoteScope } +import akka.routing.{ FromConfig, RoundRobinPool } import akka.serialization.SerializationExtension -import akka.testkit.{AkkaSpec, TestKit} +import akka.testkit.{ AkkaSpec, TestKit } import com.typesafe.config.ConfigFactory import scala.concurrent.duration._