From ccb608304519866cab63aa15a1a788a3a69a5b44 Mon Sep 17 00:00:00 2001 From: Mike Jensen Date: Mon, 10 Mar 2014 16:14:10 -0600 Subject: [PATCH] Attempting to prevent interrupting threads after future has completed. We added a 4th state, so we have "Canceling and Canceled". We are in canceling state if we previously were running, and will not transition to canceled till after the interrupt has been sent. So at the end if we are not running, or already canceled, we will sleep, waiting for the interrupt to occur so we can be sure we handle it before we let the thread complete. This also fixes a condition where we returned true on a cancel after a task has already been canceled --- .../java/util/concurrent/FutureTask.java | 45 +++++++++++++++---- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/classpath/java/util/concurrent/FutureTask.java b/classpath/java/util/concurrent/FutureTask.java index 2fb4d10e15..15d0f0161b 100644 --- a/classpath/java/util/concurrent/FutureTask.java +++ b/classpath/java/util/concurrent/FutureTask.java @@ -3,7 +3,7 @@ package java.util.concurrent; import java.util.concurrent.atomic.AtomicReference; public class FutureTask implements RunnableFuture { - private enum State { New, Canceled, Running, Done }; + private enum State { New, Canceling, Canceled, Running, Done }; private final AtomicReference currentState; private final Callable callable; @@ -41,9 +41,29 @@ public class FutureTask implements RunnableFuture { } catch (Throwable t) { failure = t; } finally { - currentState.compareAndSet(State.Running, State.Done); + if (currentState.compareAndSet(State.Running, State.Done) || + currentState.get() == State.Canceled) { + /* in either of these conditions we either were not canceled + * or we already were interrupted. The thread may or MAY NOT + * be in an interrupted status depending on when it was + * interrupted and what the callable did with the state. + */ + } else { + /* Should be in canceling state, so block forever till we are + * interrupted. If state already transitioned into canceled + * and thus thread is in interrupted status, the exception should + * throw immediately on the sleep call. + */ + try { + Thread.sleep(Long.MAX_VALUE); + } catch (InterruptedException e) { + // expected + } + } + + Thread.interrupted(); // reset interrupted status if set handleDone(); - runningThread = null; // must be set after state changed to done + runningThread = null; // must be last operation } } } @@ -66,12 +86,19 @@ public class FutureTask implements RunnableFuture { handleDone(); return true; - } else if (mayInterruptIfRunning) { - Thread runningThread = this.runningThread; - if (runningThread != null) { - runningThread.interrupt(); - - return true; + } else if (mayInterruptIfRunning && + currentState.compareAndSet(State.Running, State.Canceling)) { + // handleDone will be called from running thread + try { + Thread runningThread = this.runningThread; + if (runningThread != null) { + runningThread.interrupt(); + + return true; + } + } finally { + // we can not set to canceled until interrupt status has been set + currentState.set(State.Canceled); } }