# HG changeset patch # User chegar # Date 1413035127 -3600 # Node ID b7bdee8519d983c094c5b9dffb8912a2d80c9087 # Parent 2a1bbbaacbe8cc9e83e59e660c3ec5a5d9d02175 8060052: FutureTask; fix underflow when timeout = Long.MIN_VALUE Reviewed-by: martin diff -r 2a1bbbaacbe8 -r b7bdee8519d9 jdk/src/java.base/share/classes/java/util/concurrent/FutureTask.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 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); } diff -r 2a1bbbaacbe8 -r b7bdee8519d9 jdk/test/java/util/concurrent/FutureTask/NegativeTimeout.java --- /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 task = new FutureTask<>( () -> { return null; } ); + try { + task.get(Long.MIN_VALUE, TimeUnit.NANOSECONDS); + } catch (TimeoutException success) {} + } +} +