Making java.util.Iterator.remove() for the iterators for EnumSet more resilient (original) (raw)

Neil Richards neil.richards at ngmr.net
Thu Jan 27 11:44:20 UTC 2011

On 26 January 2011 01:57, Joshua Bloch <jjb at google.com> wrote:

I have serious reservations about this. It would be the first time (to my knowledge) that we deliberately swept a concurrent modification under the rug.  If we go to the effort of detecting it (which you're doing), the least we should do is to report it.

I see that the current API Javadoc for EnumSet currently says this about the behaviour of its iterator:

" The iterator returned by the iterator method traverses the elements in their natural order (the order in which the enum constants are declared). The returned iterator is weakly consistent: it will never throw ConcurrentModificationException and it may or may not show the effects of any modifications to the set that occur while the iteration is in progress. "

The change I suggest improves the resilience of the behaviour of the EnumSet implementations within the confines of the behaviour mandated by the existing API Javadoc.

The additional modification that you suggest would necessitate a change in the documented behaviour here, albeit to something more akin to other Iterator implementations.

I can see validity in the argument on both sides of this, but, given the way things are currently documented in the API and the ability (with the change) for the EnumSet Iterator impls to handle things in naturally graceful manner (ie. without corrupting the set), I think I still lean towards abiding by the constraints of the current documentation and so not throwing a ConcurrentModificationException. (I could probably be swayed if the consensus lay the other way, however.)

Also, this file (and most of java.util and java.lang) does not use braces on single-line if/for/while/do statements.

In using braces for single-line if statements, I was following the Java Coding Conventions, section 7.4 (http://www.oracle.com/technetwork/java/codeconventions-142311.html#449), in which it says:

" Note: if statements always use braces {}. Avoid the following error-prone form:

if (condition) //AVOID! THIS OMITS THE BRACES {}!
    statement;

"

Was I wrong to have done this? Are there any more pertinent (and/or recent(!)) coding conventions for OpenJDK ?

There was a much more serious case where we failed to throw a CME that we should have thrown, and the CCC (an internal review body within Sun, and now Oracle) ruled that it represented an unacceptable compatibility violation. If we're going to make this change to EnumSet, then we must reopen 4902078 and fix that too.

Thanks for the pointer - it seems to be an interesting problem, and one which I have not hitherto contemplated or explored.

Being generally an advocate of the philosophy of "making the word a better place, fixing one bug at a time", I don't see a solution to 4902078 as being inextricably linked with the one I'm trying to address in EnumSet here - though they're obviously in the same area of SDK function.

P.S.  There is a much more serious problem with EnumMap that we might want to fix.

I believe that the problem to which you refer is reported in 6312706, "(coll) Map entrySet iterators should return different entries on each call to next()". In the comments for that report, it suggests that the mischievous behaviour can also be seen in the entry set iterator for ConcurrentHashMap (in addition to those for IdentityHashMap and EnumMap).

It's been my intention to work on this problem, to try and brew up an suitably agreeable solution, which I'll raise in a separate thread (once I have it (!)).

PS: Thanks, Mike, for creating the RFE and posting the webrev.

-- Unless stated above: IBM email: neil_richards at uk.ibm.com IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

More information about the core-libs-dev mailing list