Ouch, this would have been introduced by me.
I will check to see how this could have passed the pre-commit regression
testing. I suspect that a regression test needs to be improved.
Mike
On Feb 24 2013, at 10:48 , Alan Bateman wrote:
> On 24/02/2013 15:49, Peter Levart wrote:
>>
>> Hi Alan,
>>
>> I checked and it seems all 3 IHM views [keySet|values|entrySet] have a
>> fail-fast iterator implementation (IdentityHashMap.IdentityHashMapIterator)
>> and all 3 are (were) using the iterator for .toArray implementations. So
>> this patch tries to preserve the behavior when there is a concurrent
>> modification (which is only possible from other thread and is illegal usage
>> anyway since IHM is not thread-safe) while executing the toArray methods on
>> the views...
>>
>> Do you see something I don't see?
> My apologies, I see it does check the modification count in
> IdentityHashMapIterator.nextIndex.
>
> However, as this forced me to looks at the changes-set again then the copy
> loop in Values.toArray has caught by eye:
>
> for (int si = 0; si < tab.length; si += 2) {
> if (tab[si++] != null) { // key present ?
> // more elements than expected -> concurrent modification
> from other thread
> if (ti >= size) {
> throw new ConcurrentModificationException();
> }
> a[ti++] = (T) tab[si]; // copy value
> }
> }
>
> Looks like si is incrementing by 3 rather than 2 (which ironically will cause
> a CME later because there will be fewer elements copied than expected).
>
> Do you concur? If so then we can create a bug to change this to test tab[si]
> and copy in tab[si+1].
>
> -Alan
