Reactor.exceptionHandler is defined on Throwabl...

Reactor.exceptionHandler is defined on Throwable instead of Exception.
Fixes potential problem with visibility of changes to a field. Review by
plocinic.

10 files changed, 41 insertions, 40 deletions

diff --git a/src/actors/scala/actors/ActorTask.scala b/src/actors/scala/actors/ActorTask.scala
index 2fa24f93af..ea8624e426 100644
--- a/src/actors/scala/actors/ActorTask.scala
+++ b/src/actors/scala/actors/ActorTask.scala
@@ -25,11 +25,13 @@ private[actors] class ActorTask(actor: Actor,
 
   protected override def beginExecution() {
     super.beginExecution()
-    if (actor.shouldExit)
-      actor.exit()
+    actor.synchronized { // shouldExit guarded by actor
+      if (actor.shouldExit)
+        actor.exit()
+    }
   }
 
-  protected override def terminateExecution(e: Exception) {
+  protected override def terminateExecution(e: Throwable) {
     val senderInfo = try { Some(actor.sender) } catch {
       case _: Exception => None
     }
@@ -41,7 +43,9 @@ private[actors] class ActorTask(actor: Actor,
 
     actor.synchronized {
       if (!actor.links.isEmpty)
-        actor exitLinked uncaught
+        actor.exitLinked(uncaught)
+      else
+        super.terminateExecution(e)
     }
   }
 
diff --git a/src/actors/scala/actors/Reactor.scala b/src/actors/scala/actors/Reactor.scala
index 85dcd57189..41f959d8f9 100644
--- a/src/actors/scala/actors/Reactor.scala
+++ b/src/actors/scala/actors/Reactor.scala
@@ -84,7 +84,11 @@ trait Reactor[Msg >: Null] extends OutputChannel[Msg] with Combinators {
    */
   def act(): Unit
 
-  protected[actors] def exceptionHandler: PartialFunction[Exception, Unit] =
+  /**
+   * This partial function is applied to `Throwable`s that propagate out of
+   * this $actor's body.
+   */
+  protected[actors] def exceptionHandler: PartialFunction[Throwable, Unit] =
     Map()
 
   protected[actors] def scheduler: IScheduler =
diff --git a/src/actors/scala/actors/ReactorTask.scala b/src/actors/scala/actors/ReactorTask.scala
index 3f9ebb6fa7..b4e09ef7f8 100644
--- a/src/actors/scala/actors/ReactorTask.scala
+++ b/src/actors/scala/actors/ReactorTask.scala
@@ -31,17 +31,17 @@ private[actors] class ReactorTask[Msg >: Null](var reactor: Reactor[Msg],
     try {
       beginExecution()
       try {
-        try {
-          if (fun eq null)
-            handler(msg)
-          else
-            fun()
-        } catch {
-          case e: Exception if (reactor.exceptionHandler.isDefinedAt(e)) =>
-            reactor.exceptionHandler(e)
-        }
+        if (fun eq null)
+          handler(msg)
+        else
+          fun()
       } catch {
         case _: KillActorControl =>
+          // do nothing
+
+        case e: Throwable if !e.isInstanceOf[SuspendActorControl] &&
+                             reactor.exceptionHandler.isDefinedAt(e) =>
+          reactor.exceptionHandler(e)
       }
       reactor.kill()
     }
@@ -49,17 +49,11 @@ private[actors] class ReactorTask[Msg >: Null](var reactor: Reactor[Msg],
       case _: SuspendActorControl =>
         // do nothing (continuation is already saved)
 
-      case e: Exception =>
-        // print message on default error stream
-        val msgException = "Uncaught exception in "+reactor+"\n"
-        val msgMessage   = if (msg != null) "Message: "+msg+"\n" else ""
-        Debug.doWarning {
-          Console.err.print(msgException + msgMessage)
-          e.printStackTrace()
-        }
-
+      case e: Throwable =>
         terminateExecution(e)
         reactor.terminated()
+        if (!e.isInstanceOf[Exception])
+          throw e
     } finally {
       suspendExecution()
       this.reactor = null
@@ -77,6 +71,9 @@ private[actors] class ReactorTask[Msg >: Null](var reactor: Reactor[Msg],
 
   protected def suspendExecution() {}
 
-  protected def terminateExecution(e: Exception) {}
+  protected def terminateExecution(e: Throwable) {
+    Console.err.println(reactor+": caught "+e)
+    e.printStackTrace()
+  }
 
 }
diff --git a/src/actors/scala/actors/UncaughtException.scala b/src/actors/scala/actors/UncaughtException.scala
index 54c28f66cf..1829d48606 100644
--- a/src/actors/scala/actors/UncaughtException.scala
+++ b/src/actors/scala/actors/UncaughtException.scala
@@ -23,7 +23,7 @@ class UncaughtException[Msg >: Null](val actor: Reactor[Msg],
                                      val message: Option[Msg],
                                      val sender: Option[OutputChannel[Any]],
                                      val thread: Thread,
-                                     cause: Exception)
+                                     cause: Throwable)
 extends Exception(cause) {
 
   override def toString() =
diff --git a/test/files/jvm/actor-exceptions.check b/test/files/jvm/actor-exceptions.check
index bd44b968cc..d86bac9de5 100644
--- a/test/files/jvm/actor-exceptions.check
+++ b/test/files/jvm/actor-exceptions.check
@@ -1,3 +1 @@
-Uncaught exception in Slave
-Message: A
-MyOtherException
+OK
diff --git a/test/files/jvm/actor-exceptions.scala b/test/files/jvm/actor-exceptions.scala
index 384226d777..75dc1e4cd5 100644
--- a/test/files/jvm/actor-exceptions.scala
+++ b/test/files/jvm/actor-exceptions.scala
@@ -19,6 +19,7 @@ object Master extends Actor {
     for (i <- 0 until 10) Slave ! A
     react {
       case Exit(from, reason) =>
+        println("OK")
     }
     } catch {
       case e: Throwable if !e.isInstanceOf[scala.util.control.ControlThrowable] =>
@@ -29,8 +30,9 @@ object Master extends Actor {
 
 object Slave extends Actor {
   override def toString = "Slave"
-  override def exceptionHandler: PartialFunction[Exception, Unit] = {
+  override def exceptionHandler: PartialFunction[Throwable, Unit] = {
     case MyException(text) =>
+    case other if !other.isInstanceOf[scala.util.control.ControlThrowable] => super.exceptionHandler(other)
   }
   def act() {
     try {
@@ -41,12 +43,14 @@ object Slave extends Actor {
           cnt += 1
           if (cnt % 2 != 0) throw MyException("problem")
           if (cnt == 10) {
-            throw new MyOtherException("unhandled")
+            throw MyOtherException("unhandled")
           }
       }
     }
     } catch {
-      case e: Throwable if !e.isInstanceOf[scala.util.control.ControlThrowable] =>
+      case e: Throwable if !e.isInstanceOf[scala.util.control.ControlThrowable] &&
+                           !e.isInstanceOf[MyException] &&
+                           !e.isInstanceOf[MyOtherException] =>
         e.printStackTrace()
     }
   }
diff --git a/test/files/jvm/actor-link-getstate.check b/test/files/jvm/actor-link-getstate.check
index 45967222e6..9755447320 100644
--- a/test/files/jvm/actor-link-getstate.check
+++ b/test/files/jvm/actor-link-getstate.check
@@ -1,5 +1,2 @@
 Done
-Uncaught exception in Master
-Message: 'done
-MyException: Master crashed
 Terminated
diff --git a/test/files/jvm/actor-uncaught-exception.check b/test/files/jvm/actor-uncaught-exception.check
index 3e669779df..2c94e48371 100644
--- a/test/files/jvm/actor-uncaught-exception.check
+++ b/test/files/jvm/actor-uncaught-exception.check
@@ -1,5 +1,2 @@
-Uncaught exception in StartError
-MyException: I don't want to run!
-Uncaught exception in MessageError
-Message: 'ping
-MyException: No message for me!
+OK
+OK
diff --git a/test/files/jvm/actor-uncaught-exception.scala b/test/files/jvm/actor-uncaught-exception.scala
index 9dbd36dd82..882362272d 100644
--- a/test/files/jvm/actor-uncaught-exception.scala
+++ b/test/files/jvm/actor-uncaught-exception.scala
@@ -43,6 +43,7 @@ object Test {
       Actor.loop {
         react {
           case Exit(actor, reason) =>
+            println("OK")
             if (actor == StartError)
               MessageError ! 'ping
             else
diff --git a/test/files/jvm/actor-uncaught-exception2.scala b/test/files/jvm/actor-uncaught-exception2.scala
index 626ce5da92..36b6f0c52e 100644
--- a/test/files/jvm/actor-uncaught-exception2.scala
+++ b/test/files/jvm/actor-uncaught-exception2.scala
@@ -58,7 +58,6 @@ object Test {
   }
 
   def main(args: Array[String]) {
-    Debug.level = 1 // decrease level so that it does not print warnings
     Supervisor.start()
   }
 }