I noticed that a spurious InterruptedErr is thrown if an Actor kills the pool it is in, and then calls Future.get():

using concurrent

class Interrupt {
    Void main() {
        pool1  := ActorPool()
        pool2  := ActorPool()

        actor1 := Actor(pool1) |->| {
            echo("Do cleanup stuff")
        }

        actor2 := Actor(pool2) |->| {
            try {
                pool2.kill              // interrupt all other actors in same pool
                actor1.send(null).get   // get() throws sys::InterruptedErr
                
            } catch (Err e)
                e.trace
        }
        
        actor2.send(null)       

        Actor.sleep(1sec)   // wait for errs to be logged
    }
}

Do cleanup stuff
sys::InterruptedErr: java.lang.InterruptedException
  java.lang.Object.wait (Object.java)
  java.lang.Object.wait (Object.java:485)
  concurrent::Future.get (Future.java:88)
  concurrent::Future.get (Future.java:75)
  acme::Interrupt.main (Interrupt.fan:16)
  fan.sys.Func$Indirect0.call (Func.java:128)
  concurrent::Actor.receive (Actor.java:101)
  concurrent::Actor._dispatch (Actor.java:230)
  concurrent::Actor._work (Actor.java:201)
  concurrent::ThreadPool$Worker.run (ThreadPool.java:262)

I hope the fix would something as simple as clearing the Java Thread's interrupted flag before wait() is called.

ActorPool.kill is defined an unorderly shutdown which kills all its threads and calls interrupt. In this case you are killing yourself, so by definition your thread is supposed to be dead and behavior becomes indeterminate. It seems like its working exactly as intended to me. You've been killed yet want to keep processing as if someone hasn't tried to kill you

If you want things to cleanup and finish processing their queue you should use ActorPool.stop. Trying out your code with stop instead of kill looks like it works as intended

Trying out your code with stop instead of kill looks like it works as intended

In the real code I want to interrupt some long timeouts, so kill() works exactly as I want.

---

by definition your thread is supposed to be dead and behavior becomes indeterminate

I understand my thread is interrupting / killing itself as well as others, but all that means is that Java's interrupted flag is set on all participating Threads.

Java methods such as Obj.wait() and Thread.sleep() will then throw an InterruptedException should that flag be set. And that is what happens when Actor.get() is called.

Therefore a simple call to clear the interrupted flag solves my issue:

using [java] java.lang::Thread

...

// interrupt this thread and all other actors in same pool
// note this thread's interrupted flag is set immediately
pool2.kill

// clear this thread's interrupted flag
Thread.interrupted

// get() completes as expected
actor1.send(null).get

So the only thing that's indeterminate is the state of Java's interrupted flag. As such it may be nice to expose a method to explicitly clear it.

Addendum:

Java's weird Interrupt semantics mean the interrupt flag is cleared when InterruptedException is thrown:

Any method that exits by throwing an InterruptedException clears interrupt status when it does so.

As such, Actor.java should re-set the interrupted flag when it catches InterruptedException and throws InterruptErr.

...
catch (InterruptedException e)
{
    // re-set the interrupted flag
    Thread.currentThread().interrupt();

    // throw Fantom Err
    throw InterruptedErr.make(e);
}

Java only clears the interrupted flag because it assumes that by catching InterruptedException you are also handling it, which in Fantom's case, is wrong - we're just converting it to a different exception.

---

As it stands right now, I can (wrongly) do this:

// interrupt this thread and all other actors in same pool
pool2.kill
future := actor1.send(null)

// implicitly clear the interrupt flag
try future.get
catch (InterruptErr e) { /* meh */ }

// call get() again, as intended
future.get

To sum up the previous posts, ActorPool.kill() invokes Java's interrupt mechanism which essentially just sets an interrupt flag. Note that InterruptedException is not guaranteed to be thrown (1). So I feel that the Actor framework could do with a little more transparency & control around the interrupt flag.

Being able to read the interrupt flag enables long running background Actors to check if they've been terminated and exit prematurely (2).

Being able to clear the interrupt flag allows an Actor to continue and perform any necessary clean up - which may involve notifying other Actors.

Given we're now exposing the interrupt flag, we just need to make sure we don't accidentally clear it (3).

References:

1. Methods that Clear the Thread.interrupt() flag
2. Implementing cancelable tasks
3. Don't swallow interrupts

If in agreement, here's a small patch that I believe does all the above:

diff -r a60c8b8decfb src/concurrent/fan/Actor.fan
--- a/src/concurrent/fan/Actor.fan	Fri Oct 06 09:21:02 2017 -0400
+++ b/src/concurrent/fan/Actor.fan	Sat Oct 07 10:39:16 2017 +0100
@@ -110,6 +110,18 @@
   protected virtual Obj? receive(Obj? msg)
 
   **
+  ** The terminated flag is set when this actor is killed via
+  ** 'ActorPool.kill()'
+  **
+  Bool isTerminated()
+
+  **
+  ** Sets or clears the terminated flag. Beneath the covers this sets
+  ** and clears Java's interrupt flag.
+  **
+  Void setTerminated(Bool val)
+
+  **
   ** Get the current number of messages pending on the message queue.
   **
   ** NOTE: this method is marked as NoDoc, it is provided for low level
diff -r a60c8b8decfb src/concurrent/java/Actor.java
--- a/src/concurrent/java/Actor.java	Fri Oct 06 09:21:02 2017 -0400
+++ b/src/concurrent/java/Actor.java	Sat Oct 07 10:39:16 2017 +0100
@@ -103,6 +103,21 @@
     return null;
   }
 
+  public final boolean isTerminated()
+  {
+    return Thread.currentThread().isInterrupted();
+  }
+
+  public final void setTerminated(boolean val)
+  {
+    if (val)
+        // set the interrupt flag
+        Thread.currentThread().interrupt();
+    else
+        // clear the interrupt flag
+        Thread.currentThread().interrupted();
+  }
+
   public final long queueSize() { return queue.size; }
 
   public final long queuePeak() { return queue.peak; }
@@ -120,6 +135,8 @@
     }
     catch (InterruptedException e)
     {
+      // re-set the interrupted flag
+      Thread.currentThread().interrupt();
       throw InterruptedErr.make(e);
     }
   }
diff -r a60c8b8decfb src/concurrent/java/ActorPool.java
--- a/src/concurrent/java/ActorPool.java	Fri Oct 06 09:21:02 2017 -0400
+++ b/src/concurrent/java/ActorPool.java	Sat Oct 07 10:39:16 2017 +0100
@@ -95,6 +95,7 @@
     }
     catch (InterruptedException e)
     {
+      // don't re-set the interrupted flag as the Actor may have been killed, but we haven't
       throw InterruptedErr.make(e);
     }
     throw TimeoutErr.make("ActorPool.join timed out");
diff -r a60c8b8decfb src/concurrent/java/Future.java
--- a/src/concurrent/java/Future.java	Fri Oct 06 09:21:02 2017 -0400
+++ b/src/concurrent/java/Future.java	Sat Oct 07 10:39:16 2017 +0100
@@ -123,6 +123,7 @@
     }
     catch (InterruptedException e)
     {
+      // don't re-set the interrupted flag as the Actor may have been killed, but we haven't
       throw InterruptedErr.make(e);
     }

Steve: the Fantom actor semantics aren't designed to mimic or map directly to Java. I think you are reading too much into the Java implementation. If we could safely kill a Java thread using Thread.stop for ActorPool.kill we would - its purpose is to shutdown the actors as quickly as possible without cleanup. The fundamental problem in your case is you are trying to do the opposite - kill yourself then continue on with your cleanup. So really you should be doing your cleanup on another actor pool or use ActorPool.stop