8060052: FutureTask; fix underflow when timeout = Long.MIN_VALUE
authorchegar
Sat, 11 Oct 2014 14:45:27 +0100
changeset 27032 b7bdee8519d9
parent 27031 2a1bbbaacbe8
child 27033 17712e486fb2
8060052: FutureTask; fix underflow when timeout = Long.MIN_VALUE Reviewed-by: martin
jdk/src/java.base/share/classes/java/util/concurrent/FutureTask.java
jdk/test/java/util/concurrent/FutureTask/NegativeTimeout.java
--- a/jdk/src/java.base/share/classes/java/util/concurrent/FutureTask.java	Sat Oct 11 13:24:23 2014 +0100
+++ b/jdk/src/java.base/share/classes/java/util/concurrent/FutureTask.java	Sat Oct 11 14:45:27 2014 +0100
@@ -163,7 +163,7 @@
 
     public boolean cancel(boolean mayInterruptIfRunning) {
         if (!(state == NEW &&
-              UNSAFE.compareAndSwapInt(this, stateOffset, NEW,
+              U.compareAndSwapInt(this, STATE, NEW,
                   mayInterruptIfRunning ? INTERRUPTING : CANCELLED)))
             return false;
         try {    // in case call to interrupt throws exception
@@ -173,7 +173,7 @@
                     if (t != null)
                         t.interrupt();
                 } finally { // final state
-                    UNSAFE.putOrderedInt(this, stateOffset, INTERRUPTED);
+                    U.putOrderedInt(this, STATE, INTERRUPTED);
                 }
             }
         } finally {
@@ -227,9 +227,9 @@
      * @param v the value
      */
     protected void set(V v) {
-        if (UNSAFE.compareAndSwapInt(this, stateOffset, NEW, COMPLETING)) {
+        if (U.compareAndSwapInt(this, STATE, NEW, COMPLETING)) {
             outcome = v;
-            UNSAFE.putOrderedInt(this, stateOffset, NORMAL); // final state
+            U.putOrderedInt(this, STATE, NORMAL); // final state
             finishCompletion();
         }
     }
@@ -245,17 +245,16 @@
      * @param t the cause of failure
      */
     protected void setException(Throwable t) {
-        if (UNSAFE.compareAndSwapInt(this, stateOffset, NEW, COMPLETING)) {
+        if (U.compareAndSwapInt(this, STATE, NEW, COMPLETING)) {
             outcome = t;
-            UNSAFE.putOrderedInt(this, stateOffset, EXCEPTIONAL); // final state
+            U.putOrderedInt(this, STATE, EXCEPTIONAL); // final state
             finishCompletion();
         }
     }
 
     public void run() {
         if (state != NEW ||
-            !UNSAFE.compareAndSwapObject(this, runnerOffset,
-                                         null, Thread.currentThread()))
+            !U.compareAndSwapObject(this, RUNNER, null, Thread.currentThread()))
             return;
         try {
             Callable<V> c = callable;
@@ -296,8 +295,7 @@
      */
     protected boolean runAndReset() {
         if (state != NEW ||
-            !UNSAFE.compareAndSwapObject(this, runnerOffset,
-                                         null, Thread.currentThread()))
+            !U.compareAndSwapObject(this, RUNNER, null, Thread.currentThread()))
             return false;
         boolean ran = false;
         int s = state;
@@ -364,7 +362,7 @@
     private void finishCompletion() {
         // assert state > COMPLETING;
         for (WaitNode q; (q = waiters) != null;) {
-            if (UNSAFE.compareAndSwapObject(this, waitersOffset, q, null)) {
+            if (U.compareAndSwapObject(this, WAITERS, q, null)) {
                 for (;;) {
                     Thread t = q.thread;
                     if (t != null) {
@@ -391,11 +389,18 @@
      *
      * @param timed true if use timed waits
      * @param nanos time to wait, if timed
-     * @return state upon completion
+     * @return state upon completion or at timeout
      */
     private int awaitDone(boolean timed, long nanos)
         throws InterruptedException {
-        final long deadline = timed ? System.nanoTime() + nanos : 0L;
+        // The code below is very delicate, to achieve these goals:
+        // - call nanoTime exactly once for each call to park
+        // - if nanos <= 0, return promptly without allocation or nanoTime
+        // - if nanos == Long.MIN_VALUE, don't underflow
+        // - if nanos == Long.MAX_VALUE, and nanoTime is non-monotonic
+        //   and we suffer a spurious wakeup, we will do no worse than
+        //   to park-spin for a while
+        long startTime = 0L;    // Special value 0L means not yet parked
         WaitNode q = null;
         boolean queued = false;
         for (;;) {
@@ -412,18 +417,30 @@
             }
             else if (s == COMPLETING) // cannot time out yet
                 Thread.yield();
-            else if (q == null)
+            else if (q == null) {
+                if (timed && nanos <= 0L)
+                    return s;
                 q = new WaitNode();
+            }
             else if (!queued)
-                queued = UNSAFE.compareAndSwapObject(this, waitersOffset,
-                                                     q.next = waiters, q);
+                queued = U.compareAndSwapObject(this, WAITERS,
+                                                q.next = waiters, q);
             else if (timed) {
-                nanos = deadline - System.nanoTime();
-                if (nanos <= 0L) {
-                    removeWaiter(q);
-                    return state;
+                final long parkNanos;
+                if (startTime == 0L) { // first time
+                    startTime = System.nanoTime();
+                    if (startTime == 0L)
+                        startTime = 1L;
+                    parkNanos = nanos;
+                } else {
+                    long elapsed = System.nanoTime() - startTime;
+                    if (elapsed >= nanos) {
+                        removeWaiter(q);
+                        return state;
+                    }
+                    parkNanos = nanos - elapsed;
                 }
-                LockSupport.parkNanos(this, nanos);
+                LockSupport.parkNanos(this, parkNanos);
             }
             else
                 LockSupport.park(this);
@@ -454,8 +471,7 @@
                         if (pred.thread == null) // check for race
                             continue retry;
                     }
-                    else if (!UNSAFE.compareAndSwapObject(this, waitersOffset,
-                                                          q, s))
+                    else if (!U.compareAndSwapObject(this, WAITERS, q, s))
                         continue retry;
                 }
                 break;
@@ -464,20 +480,17 @@
     }
 
     // Unsafe mechanics
-    private static final sun.misc.Unsafe UNSAFE;
-    private static final long stateOffset;
-    private static final long runnerOffset;
-    private static final long waitersOffset;
+    private static final sun.misc.Unsafe U;
+    private static final long STATE;
+    private static final long RUNNER;
+    private static final long WAITERS;
     static {
         try {
-            UNSAFE = sun.misc.Unsafe.getUnsafe();
+            U = sun.misc.Unsafe.getUnsafe();
             Class<?> k = FutureTask.class;
-            stateOffset = UNSAFE.objectFieldOffset
-                (k.getDeclaredField("state"));
-            runnerOffset = UNSAFE.objectFieldOffset
-                (k.getDeclaredField("runner"));
-            waitersOffset = UNSAFE.objectFieldOffset
-                (k.getDeclaredField("waiters"));
+            STATE   = U.objectFieldOffset(k.getDeclaredField("state"));
+            RUNNER  = U.objectFieldOffset(k.getDeclaredField("runner"));
+            WAITERS = U.objectFieldOffset(k.getDeclaredField("waiters"));
         } catch (Exception e) {
             throw new Error(e);
         }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/util/concurrent/FutureTask/NegativeTimeout.java	Sat Oct 11 14:45:27 2014 +0100
@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) 2014 Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8060052
+ * @summary FutureTask; fix underflow when timeout = Long.MIN_VALUE
+ * @author Chris Hegarty
+ */
+
+import java.util.concurrent.FutureTask;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.TimeUnit;
+
+// If the bug exists the test will eventually be interrupted by the
+// test harness and fail with an InterruptedException, otherwise it
+// will throw a TimeoutException almost immediately and return silently.
+
+public class NegativeTimeout {
+    public static void main(String[] args) throws Exception {
+        FutureTask<Void> task = new FutureTask<>( () -> { return null; } );
+        try {
+            task.get(Long.MIN_VALUE, TimeUnit.NANOSECONDS);
+        } catch (TimeoutException success) {}
+    }
+}
+