JDK-8349543 : LinkedBlockingDeque does not immediately throw InterruptedException in put/take
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.util.concurrent
  • Affected Version: 24
  • Priority: P4
  • Status: Closed
  • Resolution: Duplicate
  • OS: generic
  • CPU: generic
  • Submitted: 2025-02-06
  • Updated: 2025-05-13
  • Resolved: 2025-05-13
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbdResolved
Related Reports
Duplicate :  
Relates :  
Description
ADDITIONAL SYSTEM INFORMATION :
All versions

A DESCRIPTION OF THE PROBLEM :
The LinkedBlockingDeque does not behave consistently with other concurrency components. If we call putFirst(), putLast(), takeFirst(), or takeLast() with a thread that is interrupted, it does not immediately throw an InterruptedException, the way that ArrayBlockingQueue and LInkedBlockingQueue does, because instead of lockInterruptibly(), we call lock(). It will only throw an InterruptedException if the queue is full (on put) or empty (on take). Since interruptions are frequently used as a shutdown mechanism, this might prevent code from ever shutting down.

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
Call putFirst() on a LinkedBlockingDeque with an interrupted thread.

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
Expected an InterruptedException
ACTUAL -
The methods succeeed.

---------- BEGIN SOURCE ----------
import java.util.concurrent.*;

public class InterruptionsIgnoredOnPutLBD {
    public static void main(String... args) throws InterruptedException {
        // Ensure that putFirst(), putLast(), takeFirst(), and takeLast()
        // immediately throw an InterruptedException if the thread is
        // interrupted, to be consistent with other blocking queues such as
        // ArrayBlockingQueue and LinkedBlockingQueue
        try (var pool = Executors.newSingleThreadExecutor()) {
            Future<Void> success = pool.submit(() -> {
                var queue = new LinkedBlockingDeque<>();
                Thread.currentThread().interrupt();
                try {
                    queue.putFirst(42);
                    fail("Expected InterruptedException in putFirst()");
                } catch (InterruptedException expected) {
                    // good that's what we want
                }

                Thread.currentThread().interrupt();
                try {
                    queue.putLast(42);
                    fail("Expected InterruptedException in putLast()");
                } catch (InterruptedException expected) {
                    // good that's what we want
                }

                queue.add(42);
                Thread.currentThread().interrupt();
                try {
                    queue.takeFirst();
                    fail("Expected InterruptedException in takeFirst()");
                } catch (InterruptedException expected) {
                    // good that's what we want
                }

                queue.add(42);
                Thread.currentThread().interrupt();
                try {
                    queue.takeLast();
                    fail("Expected InterruptedException in takeLast()");
                } catch (InterruptedException expected) {
                    // good that's what we want
                }
                return null;
            });
            try {
                success.get();
            } catch (ExecutionException e) {
                try {
                    throw e.getCause();
                } catch (Error | RuntimeException unchecked) {
                    throw unchecked;
                } catch (Throwable cause) {
                    throw new AssertionError(cause);
                }
            }
        }
    }

    private static void fail(String message) {
        throw new AssertionError(message);
    }
}

Comments
One of the things I noticed when writing my latest Java Specialists' Newsletter (https://www.javaspecialists.eu/archive/Issue322-Throwing-InterruptedException-Early.html) was that LBD throws InterruptedException early for the timed poll() and offer() methods, but not for take() and put(). Even if there are other approaches in some of the other classes, perhaps we should fix the LBD to at least be internally consistent? import java.util.concurrent.*; public class OfferPollLBD { public static void main(String... args) { var lbd = new LinkedBlockingDeque<Integer>(); Thread.currentThread().interrupt(); System.out.println(lbd.offer(1)); // should work System.out.println(lbd.add(2)); // should work try { System.out.println(lbd.offer(3, 10, TimeUnit.SECONDS)); // throws IE } catch (InterruptedException e) { System.out.println("Interrupted"); Thread.currentThread().interrupt(); } try { lbd.put(4); // IMHO should throw IE, but doesn't System.out.println("put(4) succeeded"); } catch (InterruptedException e) { System.out.println("Interrupted"); Thread.currentThread().interrupt(); } System.out.println("lbd = " + lbd); System.out.println(lbd.poll()); // should work System.out.println(lbd.remove()); // should work try { System.out.println(lbd.poll(10, TimeUnit.SECONDS)); // throws IE } catch (InterruptedException e) { System.out.println("Interrupted"); Thread.currentThread().interrupt(); } try { System.out.println(lbd.take()); } catch (InterruptedException e) { System.out.println("Interrupted"); Thread.currentThread().interrupt(); } System.out.println("lbd = " + lbd); } }
10-03-2025

We tried to generally word this so that IE only had to be thrown if the thread was interrupted "whilst waiting/blocking". As Object.wait() is unconditionally going to block it has to detect the IE and rethrow it. The immediacy of that (did it do it before the monitor was released, or sometime after) is not readily observable. For other API's interruption is more clearly only checked once we have determined that we will have to block. Any API that throws IE can choose when it actually checks for it.
11-02-2025

I now see that the unconditionally unbounded BlockingQueue.put implementations (like DelayQueue) do not declare to throw IE, and changing that would be too great an incompatibility. So these methods can never throw an "early" IE. POLA suggests: - users can expect the dynamically optionally unbounded implementations LBD and LBQ to work the same way, i.e. also never throw IE when unbounded - users can expect LBD to work the same way as LBQ. The only way I see that we can fully follow POLA is to eliminate all early IE. (I'm not advocating that)
10-02-2025

I started going through the other concurrency constructs to see whether any others have behaviour like the LinkedBlockingDeque. Constructs like CyclicBarrier, CountDownLatch, Semaphore, Phaser, the ReentrantLock and ReentrantReadWriteLock, the AbstractQueuedSynchronizers, StampedLock are consistent with LinkedBlockingQueue, throwing an InterruptedException immediately upon entry to interruptible methods. BTW, this is also consistent with your Object.wait() works - if the thread is interrupted, the monitor is not released, but instead, the InterruptedException is immediately thrown. However, I found a few places that do not behave liked this. For example: Exchanger.exchange() - if the value is already there, then no InterruptedException is thrown ForkJoinPool / ThreadPoolExecutor.awaitTermination() - if the pool is already terminated, no InterruptedException is thrown. There might be more places that don't immediately exit the interruptible methods. I bumped into the LBD issue when I was trying to shut down some code using interruptions and it wasn't returning. If I had a vote, I would vote to change just the LinkedBlockingDeque to be consistent with LinkedBlockingQueue, because otherwise we are breaking the principle of least astonishment (POLA).
10-02-2025

I dusted off some of the old test infrastructure, extended the testing across classes and doing both put/take: diff --git a/test/jdk/java/util/concurrent/tck/BlockingQueueTest.java b/test/jdk/java/util/concurrent/tck/BlockingQueueTest.java index 101b583ccab..294d6f77659 100644 --- a/test/jdk/java/util/concurrent/tck/BlockingQueueTest.java +++ b/test/jdk/java/util/concurrent/tck/BlockingQueueTest.java @@ -398,6 +398,32 @@ public void testRemoveElement() { checkEmpty(q); } + /** + * take() throws InterruptedException if interrupted before waiting, even + * if elements are available. + */ + public void testTakePreInterruptedNotEmpty() { + final BlockingQueue q = emptyCollection(); + if (q.remainingCapacity() == 0) return; // SynchronousQueue + q.add(makeElement(42)); + Thread.currentThread().interrupt(); + assertThrows(InterruptedException.class, + () -> q.take()); + } + + /** + * put(x) throws InterruptedException if interrupted before waiting, even + * if capacity is available. + */ + public void testPutPreInterruptedNotFull() { + final BlockingQueue q = emptyCollection(); + if (q.remainingCapacity() == 0) return; // SynchronousQueue + Object x = makeElement(42); + Thread.currentThread().interrupt(); + assertThrows(InterruptedException.class, + () -> q.put(x)); + } + /** For debugging. */ public void XXXXtestFails() { fail(emptyCollection().getClass().toString()); jtreg gives me (EDIT: SEE LATER COMMENT FOR A BETTER WAY TO RUN TESTS) $ /home/martin/jtreg-binaries/current/bin/jtreg -noreport -agentvm -w:/tmp/jtr-Xcb2Qy -nativepath:/home/martin/ws/jdk/build/linux-x86_64-server-release/images/jdk/../test/jdk/jtreg/native -verbose:nopass,fail,error -vmoption:-enablesystemassertions -automatic -ignore:quiet -jdk:/home/martin/ws/jdk/build/linux-x86_64-server-release/images/jdk . |& grep '^FAILED' FAILED LinkedTransferQueueTest$Generic::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED DelayQueueTest$Generic::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED LinkedBlockingDequeTest$Unbounded::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED LinkedBlockingDequeTest$Unbounded::testTakePreInterruptedNotEmpty 'testTakePreInterruptedNotEmpty' FAILED LinkedBlockingDequeTest$Bounded::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED LinkedBlockingDequeTest$Bounded::testTakePreInterruptedNotEmpty 'testTakePreInterruptedNotEmpty' FAILED PriorityBlockingQueueTest$Generic::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED PriorityBlockingQueueTest$InitialCapacity::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED LinkedTransferQueueTest$Generic::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED DelayQueueTest$Generic::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED LinkedBlockingDequeTest$Unbounded::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED LinkedBlockingDequeTest$Unbounded::testTakePreInterruptedNotEmpty 'testTakePreInterruptedNotEmpty' FAILED LinkedBlockingDequeTest$Bounded::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED LinkedBlockingDequeTest$Bounded::testTakePreInterruptedNotEmpty 'testTakePreInterruptedNotEmpty' FAILED PriorityBlockingQueueTest$Generic::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED PriorityBlockingQueueTest$InitialCapacity::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED LinkedTransferQueueTest$Generic::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED DelayQueueTest$Generic::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED LinkedBlockingDequeTest$Unbounded::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED LinkedBlockingDequeTest$Unbounded::testTakePreInterruptedNotEmpty 'testTakePreInterruptedNotEmpty' FAILED LinkedBlockingDequeTest$Bounded::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED LinkedBlockingDequeTest$Bounded::testTakePreInterruptedNotEmpty 'testTakePreInterruptedNotEmpty' FAILED PriorityBlockingQueueTest$Generic::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED PriorityBlockingQueueTest$InitialCapacity::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull'
10-02-2025

Good thing that my former self used BlockingQueueTest.java with both bounded and unbounded variants ... I had forgotten about those. The difference in behavior between bounded and unbounded queues seems quite artificial. Why should an unbounded queue behave differently from a queue with a very large capacity? You might consider a "put" method that happens to be declared to throw IE because its queue is bounded, and then throws IE even when it is not full, is taking unfair advantage of its declared checked exception! The only big-picture consistent behavior for all methods in all collections is to check for interrupts only when about to block, but that's not where the proposed change in this bug is taking us. Any user who would like aggressive interrupt checking can do so themselves in their own code, possibly by subclassing j.u.c. classes.
10-02-2025

Thanks [~hkabutz] for tickling a few more memory cells. Here's another possible testPutPreInterruptedNotFull, that - uses (q.remainingCapacity() == Integer.MAX_VALUE) to check for (dynamic) unboundedness - checks that IE is thrown iff bounded I decided to keep the SynchronousQueue check even though it could be removed since this test is all about calling put when capacity is available /** * put(x) throws InterruptedException if interrupted before waiting, even * if capacity is available. Unless ... unbounded?! */ public void testPutPreInterruptedNotFull() throws InterruptedException { final BlockingQueue q = emptyCollection(); if (q.remainingCapacity() == 0) return; // SynchronousQueue Object x = makeElement(42); Thread.currentThread().interrupt(); if (q.remainingCapacity() == Integer.MAX_VALUE) { // unbounded q.put(x); Thread.interrupted(); // clear interrupt status } else { assertThrows(InterruptedException.class, () -> q.put(x)); } } That results in these failures: FAILED LinkedBlockingDequeTest$Bounded::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' FAILED LinkedBlockingQueueTest$Unbounded::testPutPreInterruptedNotFull 'testPutPreInterruptedNotFull' The unbounded LinkedBlockingQueue throws IE while the bounded LinkedBlockingDeque does not.
10-02-2025

I realized I could run just one instance of these tests by passing the test name 'JSR166TestCase.java#default' My current testing command line looks like /home/martin/jtreg-binaries/current/bin/jtreg -noreport -agentvm -w:$(mktemp --tmpdir -d jtreg-XXXX) -verbose:nopass,fail,error -automatic -ignore:quiet -jdk:/home/martin/ws/jdk/build/linux-x86_64-server-release/images/jdk 'JSR166TestCase.java#default'
10-02-2025

Thanks for looking at this Martin Buchholz. The put() methods for DelayQueue, LinkedTransferQueue, and PriorityBlockingQueue do not declare that they throw InterruptedException, since they are always unbounded and thus put() would never block. The test could perhaps filter away any BlockingQueue where the put() method does not declare that it throws an InterruptedException? For example: final BlockingQueue q = emptyCollection(); if (q.remainingCapacity() == 0) return; // SynchronousQueue Class putParameterType = (Class) q.getClass().getTypeParameters()[0].getBounds()[0]; // Class putParameterType = q.getClass().getMethod("take").getReturnType(); // this also works if (!Set.of(q.getClass().getMethod("put", putParameterType) .getExceptionTypes()).contains( InterruptedException.class)) return; // unbounded BlockingQueue Object x = makeElement(42); Thread.currentThread().interrupt(); ... We can include the put() test for the SynchronousQueue, because that also immediately throws InterruptedException if we call it with an interrupted thread.
10-02-2025

More reconstruction of 10-year-old thoughts... Nobody likes the current inconsistency of behavior. It's easy to see how natural coding of these methods, (including third-party), is likely to lead to the current state. But are these bugs, or suboptimal implementation, or warts we should continue to live with? If we decide to fix, then: - javadoc should be updated to reflect that decision. - my tests from another comment should be expanded to cover all implementations and methods in openjdk - all resulting failures from those tests should be fixed. It's a big job, one I decided not to tackle long ago. OTOH openjdk is more open to potentially incompatible changes these days.
09-02-2025

Java Historian tried to page in some failing memory and read between the tests: BlockingQueueTest.java:286: public void testTakeFromEmptyBlocksInterruptibly() { BlockingQueueTest.java:309: public void testTakeFromEmptyAfterInterrupt() { BlockingQueueTest already very carefully tests interrupts before and after invoking take(), but only when there is nothing to take(). Probably a test was written for the non-empty case (like in a comment here!), but was abandoned because it was deemed too much legacy inconsistency to be worth the effort fixing (back in 2016) ... to be rediscovered in this bug report.
06-02-2025

My testing suggests that LinkedBlockingDeque.take() is not as much of an outlier as believed. We should decide whether it's worth making openjdk behavior of these classes consistent. If so, we should expand my tests from the previous comment to include more classes and/or methods. Java Historian can't remember whether this particular class of inconsistency has been noticed before.
06-02-2025

A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk/pull/23464 Date: 2025-02-05 15:36:15 +0000
06-02-2025

Tests: 24 - Fail 23.0.1 - Fail 21.0.5 - Fail 17.0.13 - Fail 11.0.22 - Fail 11.0.13 - Fail 11.0.1 - Fail
06-02-2025