“Copy-on-Iterate” Java Idiom considered broken

Discussions

News: “Copy-on-Iterate” Java Idiom considered broken

  1. One issue with collections is that when iterating a ConcurrentModificationException is thrown if a collection is changed (usually by a different thread). This is done to protect the Iterator from unpredictable behaviour. A common idiom to avoid it is to copy the collection before iteration like that:

    for(Iterator i = new ArrayList(collection).iterator(); i.hasNext();) {...}

    Note that the collection must either to be synchronized or otherwise suitable for a multi-threaded environment.

    This idiom is a very common one, as easily proven by a simple Google Code search. In fact we used it several times in the JRebel code and it is present in several places in Wicket as well. So how could this cause an ArrayIndexOutOfBoundsException? Find out in our article:

    http://www.zeroturnaround.com/blog/copy-on-iterate-java-idiom-considered-broken/


  2. This is really good information.  Thanks for sharing it with the community.

    It appears to me, however, that the ArrayList constructor contained a bug, not that the idiom is broken.  One could hack a class together to avoid the bug for this purpose.   And on 1.6, then there is no issue.

    This line of the ArrayList constructor:

    collection.toArray(array);

    Needs to be changed to:

    array = collection.toArray(array);

  3. This line of the ArrayList constructor:

    collection.toArray(array);

    Needs to be changed to:

    array = collection.toArray(array);

    Sorry.  That's wrong.  This is the correct answer:

    array = collection.toArray();

    And in 1.5 an above, a simple fix is to do this instead of creating a new collection and iterator:

    for (Foo f : toCopy.toArray())

     

  4. You're just using really old software.

    Sam

  5. right back at you[ Go to top ]

    And the same could be said for your choice in application performance monitoring tools :-)

  6. AFAIK no "idiom" whatsoever breaks here.

    It's just the combination of a concurrent collection and a non-thread-safe collection, that leads to non-thread-safe code.

    But ok, "Mixups of concurrent and not-threadsafe code-idiom in Java broken" wouldn't be a hell of heading for a blog post, wouldn't it?