Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-6258302

(coll) LinkedHashIterator doesn't detect ConcurrentModificationEx during last elt

    Details

    • Type: Bug
    • Status: Closed
    • Priority: P4
    • Resolution: Won't Fix
    • Affects Version/s: 5.0
    • Fix Version/s: None
    • Component/s: core-libs
    • Labels:

      Description

      FULL PRODUCT VERSION :
      java version "1.4.2"
      Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.2-b28)
      Java HotSpot(TM) Client VM (build 1.4.2-b28, mixed mode)

      (Also checked 1.5.0 sources)

      ADDITIONAL OS VERSION INFORMATION :
      Windows NT Version 4.0

      A DESCRIPTION OF THE PROBLEM :
      The LinkedHashIterator, defined in java.util.LinkedHashMap, does not properly detect and throw a ConcurrentModificationException when iterating on the last element in the list. If items are added while processing elements (1..n-1), it is thrown. But if while processing the last element, items are added, the Exception is not decected.

      The Iterator for LinkedHashSet is LinkedHashMap.LinkedHashIterator. In LinkedHashIterator.nextEntry(), the last thing it does before returning the Entry e is to set a member var nextEntry to e.after. At the following call to hasNext(), it checks nextEntry != header.

      So, if nextEntry() is currently returning the last element in the list, its .after is presently header. Now, if in processing the last element, something gets added to the list, the value being checked by hasNext() is the old nextEntry.after (which was the header but now has a value.) hasNext() should be looking at the current value of .after, not what it was last time nextEntry() was called. (perhaps it needs to instead look at lastReturned.after?) This would allow hasNext() to return true, then the subsequent nextEntry() to correctly detect and throw a ConModEx.

      Actually, since the LinkedHashSet only adds to the end, I think nextEntry()'s condition for throwing a ConModEx should be (modCount < expectedModCount), which would detect removes but allow for adds.

      STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
      See program source.

      EXPECTED VERSUS ACTUAL BEHAVIOR :
      EXPECTED -
      1
      2
      Caught expected ConcurrentModificationException
      1
      2
      3
      4
      Caught expected ConcurrentModificationException


      Actually, I debate whether an add() should trigger a ConModEx. I believe it's a safe action, in which case, I'd prefer to see:
      1
      2
      3
      4
      1
      2
      3
      4
      5
      with no Exceptions thrown.

      ACTUAL -
      1
      2
      Caught expected ConcurrentModificationException
      1
      2
      3
      4
      Failed to catch expected ConcurrentModificationException

      REPRODUCIBILITY :
      This bug can be reproduced always.

      ---------- BEGIN SOURCE ----------
      import java.util.ConcurrentModificationException;
      import java.util.LinkedHashSet;
      import java.util.Iterator;

      public class LHSBug
      {
        public LHSBug()
        {
        }
        
        public static void main(String[] args)
        {
          LinkedHashSet lhSet = new LinkedHashSet();
          Integer one = new Integer(1);
          Integer two = new Integer(2);
          Integer three = new Integer(3);
          Integer four = new Integer(4);
          Integer cinco = new Integer(5);
          
          // Add the three objects to the LinkedHashSet.
          // By its nature, the LinkedHashSet will iterate in
          // order of insertion.
          lhSet.add(one);
          lhSet.add(two);
          lhSet.add(three);
          
          // 1. Iterate over set. try to insert while processing the
          // second item. This should throw a ConcurrentModificationEx
          try
          {
            for (Iterator it = lhSet.iterator(); it.hasNext(); )
            {
              Integer num = (Integer) it.next();
              if (num == two)
              {
                lhSet.add(four);
              }
              System.out.println(num);
            }
          }
          catch (ConcurrentModificationException ex)
          {
            System.out.println("Caught expected ConcurrentModificationException");
          }
          
          // 2. Iterate again, this time inserting on the (old) 'last'
          // element. This too should throw a ConcurrentModificationEx.
          // But it doesn't.
          try
          {
            for (Iterator it = lhSet.iterator(); it.hasNext(); )
            {
              Integer num = (Integer) it.next();
              if (num == four)
              {
                lhSet.add(cinco);
              }
              System.out.println(num);
            }
            
            // The exception isn't detected in this case because the
            // LinkedHashIterator.nextEntry(), when processing elt four,
            // sets nextEntry = e.after. After the .next(), but before the
            // add, this is header. But the add() changes the .after for
            // elt four. But this isn't detected, because the following call
            // to hasNext() checks the previously stored value of nextEntry.
            System.out.println("Failed to catch expected ConcurrentModificationException");
          }
          catch (ConcurrentModificationException ex)
          {
            System.out.println("Caught expected ConcurrentModificationException");
          }
          
          // Actually, I question why an add should trigger a ConModEx.
          // Since the Iterator is traversing the linked list, and items
          // are only added to the end of the list, an add should be safe.

        }
      }
      ---------- END SOURCE ----------

      CUSTOMER SUBMITTED WORKAROUND :
      Either avoid using a LinkedHashSet.iterator() and get by index, or stop using LinkedHashSet and use another collection (both of which have performance implications)
      ###@###.### 2005-04-20 11:06:41 GMT

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                Unassigned
                Reporter:
                ndcosta Nelson Dcosta (Inactive)
              • Votes:
                0 Vote for this issue
                Watchers:
                0 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Imported:
                  Indexed: