JDK-6993789 : LinkedBlockingDeque iterator/descendingIterator loops and owns lock forever
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.util.concurrent
  • Affected Version: 6u22
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: linux
  • CPU: x86
  • Submitted: 2010-10-21
  • Updated: 2014-07-03
  • Resolved: 2011-03-08
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.
JDK 6 JDK 7
6u85Fixed 7 b120Fixed
Related Reports
Relates :  
Description
FULL PRODUCT VERSION :
java version "1.6.0_21"
Java(TM) SE Runtime Environment (build 1.6.0_21-b06)
Java HotSpot(TM) Server VM (build 17.0-b16, mixed mode)

Same with build 1.7.0-ea-b114

ADDITIONAL OS VERSION INFORMATION :
Linux ... 2.6.34.7-0.3-desktop #1 SMP PREEMPT 2010-09-20 15:27:38 +0200 i686 i686 i386 GNU/Linux

A DESCRIPTION OF THE PROBLEM :
The iterator of the LinkedBlockingDeque can loop for ever in a particular case when attempting to retrieve the next item to return. As the iterator owns the deque's lock, the deque becomes unusable. Indeed, this blocks all threads attempting to access the deque and the thread that uses the iterator.

The test case that reproduces the issue follows but here is what is happening (switch to monospace for the illustration).

- Add three elements in the deque: "1", "2", "3"
- Create an iterator and iterate just once. The iterator has returned the "1" item and is internally referencing the node of the "2".
- Remove in that order "2", "1", "3". The implementation of the unlinkFirst() method sets the next reference of the node removed to itself. Thus, nodes "1" and "3" both have their next references to themselves. Here is the illustration of the iterator


        +-----+                 +-----+
        |     |                 |     |
        v     |                 v     |
     +-----+  |  +-----+     +-----+  |
     |     |--+  |     |---->|     |--+
     |  1  |     |  2  |     |  3  |
  X<-|     |<----|     |  X<-|     |
     +-----+     +-----+     +-----+
        |           |           |
        v           v           v
        X           X           X


- From that situation, the iterator loops forever.



REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
    Deque<String> deque = new LinkedBlockingDeque<String>();
    deque.add("1");
    deque.add("2");
    deque.add("3");

    int i=1;

    for(String s:deque)
    {
      System.out.println(s);
      if ( i == 1 )
      {
        deque.remove("2");
        deque.remove("1");
        deque.remove("3");
      }
      
      i++;
    }
---------- END SOURCE ----------

Comments
EVALUATION http://hg.openjdk.java.net/jdk7/build/jdk/rev/bf284d2db008
04-12-2010

EVALUATION Changeset: bf284d2db008 Author: chegar Date: 2010-11-15 15:11 +0000 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/bf284d2db008 6993789: LinkedBlockingDeque iterator/descendingIterator loops and owns lock forever Reviewed-by: dl, dholmes ! src/share/classes/java/util/concurrent/LinkedBlockingDeque.java ! test/java/util/concurrent/ConcurrentQueues/IteratorWeakConsistency.java
15-11-2010

EVALUATION descendingIterator suffers from a similar problem: deque.add("1"); deque.add("2"); deque.add("3"); deque.add("4"); remove = true; Iterator<String> descItr = deque.descendingIterator(); while (descItr.hasNext()) { String s = descItr.next(); System.out.println(s); if (remove) { deque.remove("3"); deque.remove("4"); deque.remove("2"); remove = false; } }
05-11-2010

PUBLIC COMMENTS We start with the list: 1, 2, 3. We create an iterator that initially refers to 1. We access 1 through the iterator and iterator advances to point to 2. We remove 2, which remains linked between 1 and 3. We remove 1, which leaves 1 linked to itself and the head advances to 3. We remove 3, which links to itself and head is now null. We loop around and try to access "next" which is 2 and that calls advance() - next moves to 3 which is linked to itself and we enter the infinite loop. It is not clear how to resolve this while still keeping the weakly-consistent guarantee of the iterator. The original logic was intended to skip deleted nodes, but assumed you could find the non-deleted nodes - but now they are unlinked you can't find them.
03-11-2010

EVALUATION This change regression was introduced by the fix for 6805775 "LinkedBlockingQueue Nodes should unlink themselves before becoming garbage". The problem is in AbstractItr advance: 1057 void advance() { 1058 final ReentrantLock lock = LinkedBlockingDeque.this.lock; 1059 lock.lock(); 1060 try { 1061 // assert next != null; 1062 Node<E> s = nextNode(next); 1063 if (s == next) { 1064 next = firstNode(); 1065 } else { 1066 // Skip over removed nodes. 1067 // May be necessary if multiple interior Nodes are emoved. 1068 while (s != null && s.item == null) 1069 s = nextNode(s); 1070 next = s; 1071 } 1072 nextItem = (next == null) ? null : next.item; 1073 } finally { 1074 lock.unlock(); 1075 } 1076 } The while loop at 1068 becomes an infinite loop if the Node has been removed and so node.next == node.
03-11-2010