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

[Mike Duigou]

After thinking about it for a few days and talking the issue over with Alan Bateman I suspect that Josh's fix would be rejected by CCC on the same grounds as CR #4902078--a ConcurrentModificationException would be thrown where previously none was thrown. This case is also probably less ambiguous than 4902078 as currently CME is never thrown by EnumSet iterators.

Neil's fix though might be acceptable in that improves the behaviour of the class and avoids corruption of the object state which can currently result from concurrent removes().

My preference, given a time machine, would be for consistency with the other iterators which throw CME. Lacking that, I believe that adding CME to EnumSet iterator() will be rejected over compatibility concerns.

On Jan 27 2011, at 03:44 , Neil Richards wrote:

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

I have re-opened 6312706 (I closed it) with this clarification and added both of you to the interest list.

Mike