[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-core-libs-dev
Subject:    Making java.util.Iterator.remove() for the iterators for EnumSet more resilient
From:       neil.richards () ngmr ! net (Neil Richards)
Date:       2011-01-27 11:44:20
Message-ID: AANLkTikdLy-QR-P+FvwqAWLYA4eoUc1TNSCAJPeio7sB () mail ! gmail ! com
[Download RAW message or body]

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 (!)).

- Neil

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

[prev in list] [next in list] [prev in thread] [next in thread]