From 43dc3f79923a70306eaf91f77392b7dbb99f1fd1 Mon Sep 17 00:00:00 2001 From: Martin Buchholz Date: Sun, 13 Dec 2020 19:17:36 +0000 Subject: [PATCH] 8254350: CompletableFuture.get may swallow InterruptedException Reviewed-by: alanb, dl --- .../util/concurrent/CompletableFuture.java | 18 ++-- .../CompletableFuture/LostInterrupt.java | 73 +++++++++++++++ .../SwallowedInterruptedException.java | 90 +++++++++++++++++++ 3 files changed, 173 insertions(+), 8 deletions(-) create mode 100644 test/jdk/java/util/concurrent/CompletableFuture/LostInterrupt.java create mode 100644 test/jdk/java/util/concurrent/CompletableFuture/SwallowedInterruptedException.java diff --git a/src/java.base/share/classes/java/util/concurrent/CompletableFuture.java b/src/java.base/share/classes/java/util/concurrent/CompletableFuture.java index 2e704af9b957e..fbb34e84756d0 100644 --- a/src/java.base/share/classes/java/util/concurrent/CompletableFuture.java +++ b/src/java.base/share/classes/java/util/concurrent/CompletableFuture.java @@ -1871,6 +1871,8 @@ public boolean block() { * interrupted. */ private Object waitingGet(boolean interruptible) { + if (interruptible && Thread.interrupted()) + return null; Signaller q = null; boolean queued = false; Object r; @@ -1882,25 +1884,25 @@ private Object waitingGet(boolean interruptible) { } else if (!queued) queued = tryPushStack(q); + else if (interruptible && q.interrupted) { + q.thread = null; + cleanStack(); + return null; + } else { try { ForkJoinPool.managedBlock(q); } catch (InterruptedException ie) { // currently cannot happen q.interrupted = true; } - if (q.interrupted && interruptible) - break; } } - if (q != null && queued) { + if (q != null) { q.thread = null; - if (!interruptible && q.interrupted) + if (q.interrupted) Thread.currentThread().interrupt(); - if (r == null) - cleanStack(); } - if (r != null || (r = result) != null) - postComplete(); + postComplete(); return r; } diff --git a/test/jdk/java/util/concurrent/CompletableFuture/LostInterrupt.java b/test/jdk/java/util/concurrent/CompletableFuture/LostInterrupt.java new file mode 100644 index 0000000000000..0f5a462b630d1 --- /dev/null +++ b/test/jdk/java/util/concurrent/CompletableFuture/LostInterrupt.java @@ -0,0 +1,73 @@ +/* + * Copyright (c) 2020, 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. + */ + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ForkJoinPool; +import java.util.concurrent.ThreadLocalRandom; +import static java.util.concurrent.TimeUnit.DAYS; + +/* + * @test + * @bug 8254350 + * @run main LostInterrupt + * @summary CompletableFuture.get may swallow interrupt status + * @key randomness + */ + +// TODO: Rewrite as a CompletableFuture tck test ? + +/** + * Submits a task that completes immediately, then invokes CompletableFuture.get + * with the interrupt status set. CompletableFuture.get should either complete + * immediately with the interrupt status set, or else throw InterruptedException + * with the interrupt status cleared. + */ +public class LostInterrupt { + static final int ITERATIONS = 10_000; + + public static void main(String[] args) throws Exception { + ThreadLocalRandom rnd = ThreadLocalRandom.current(); + ForkJoinPool executor = new ForkJoinPool(1); + try { + for (int i = 0; i < ITERATIONS; i++) { + CompletableFuture future = new CompletableFuture<>(); + boolean timed = rnd.nextBoolean(); + executor.execute(() -> future.complete("foo")); + + Thread.currentThread().interrupt(); + try { + String result = timed ? future.get(1, DAYS) : future.get(); + + if (!Thread.interrupted()) + throw new AssertionError("lost interrupt, run=" + i); + } catch (InterruptedException expected) { + if (Thread.interrupted()) + throw new AssertionError( + "interrupt status not cleared, run=" + i); + } + } + } finally { + executor.shutdown(); + } + } +} diff --git a/test/jdk/java/util/concurrent/CompletableFuture/SwallowedInterruptedException.java b/test/jdk/java/util/concurrent/CompletableFuture/SwallowedInterruptedException.java new file mode 100644 index 0000000000000..21097fb64c6c9 --- /dev/null +++ b/test/jdk/java/util/concurrent/CompletableFuture/SwallowedInterruptedException.java @@ -0,0 +1,90 @@ +/* + * Copyright (c) 2020, 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. + */ + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.atomic.AtomicReference; + +/* + * @test + * @bug 8254350 + * @run main SwallowedInterruptedException + * @key randomness + */ + +public class SwallowedInterruptedException { + static final int ITERATIONS = 100; + + public static void main(String[] args) throws Throwable { + for (int i = 1; i <= ITERATIONS; i++) { + System.out.format("Iteration %d%n", i); + + CompletableFuture future = new CompletableFuture<>(); + CountDownLatch running = new CountDownLatch(1); + AtomicReference failed = new AtomicReference<>(); + + Thread thread = new Thread(() -> { + // signal main thread that child is running + running.countDown(); + + // invoke Future.get, it complete with the interrupt status set or + // else throw InterruptedException with the interrupt status not set. + try { + future.get(); + + // interrupt status should be set + if (!Thread.currentThread().isInterrupted()) { + failed.set("Future.get completed with interrupt status not set"); + } + } catch (InterruptedException ex) { + // interrupt status should be cleared + if (Thread.currentThread().isInterrupted()) { + failed.set("InterruptedException with interrupt status set"); + } + } catch (Throwable ex) { + failed.set("Unexpected exception " + ex); + } + }); + thread.setDaemon(true); + thread.start(); + + // wait for thread to run + running.await(); + + // interrupt thread and set result after an optional (random) delay + thread.interrupt(); + long sleepMillis = ThreadLocalRandom.current().nextLong(10); + if (sleepMillis > 0) + Thread.sleep(sleepMillis); + future.complete(null); + + // wait for thread to terminate and check for failure + thread.join(); + String failedReason = failed.get(); + if (failedReason != null) { + throw new RuntimeException("Test failed: " + failedReason); + } + } + } +}