Re: FutureTask.cancel(true) should run thread.interrupt within doPrivileged

[Peter Levart]

Hi Martin,

If you want to optimize for without-security-manager case, then it would 
be better this way:

    private static void privilegedInterrupt(Thread t) {
        if (System.getSecurityManager() == null) {
            t.interrupt();
        } else {
            PrivilegedAction<Void> doInterrupt =
                () -> { t.interrupt(); return null; };
           AccessController.doPrivileged(doInterrupt);
        }
    }


...and no security exception catching. This way you don't trigger double 
security checks for case with-security-manager and not enough permission...

Regards, Peter

On 10/02/2013 06:29 PM, Martin Buchholz wrote:
FutureTask.cancel(true) invokes thread.interrupt on the thread (if any)
currently running the task.
This should succeed even if modifyThread permission is denied by the
security manager.

Here's a proposed fix for jdk8+:

--- src/main/java/util/concurrent/FutureTask.java 15 May 2013 02:39:59 -0000
1.103
+++ src/main/java/util/concurrent/FutureTask.java 2 Oct 2013 16:25:23 -0000
@@ -132,6 +132,12 @@
          return state != NEW;
      }

+    private static void privilegedInterrupt(Thread t) {
+        java.security.PrivilegedAction<Void> doInterrupt =
+            () -> { t.interrupt(); return null; };
+        java.security.AccessController.doPrivileged(doInterrupt);
+    }
+
      public boolean cancel(boolean mayInterruptIfRunning) {
          if (!(state == NEW &&
                UNSAFE.compareAndSwapInt(this, stateOffset, NEW,
@@ -142,7 +148,11 @@
                  try {
                      Thread t = runner;
                      if (t != null)
-                        t.interrupt();
+                        try {
+                            t.interrupt();
+                        } catch (SecurityException e) {
+                            privilegedInterrupt(t);
+                        }
                  } finally { // final state
                      UNSAFE.putOrderedInt(this, stateOffset, INTERRUPTED);
                  }