Reputation: 11150
When we remove -1
from and empty ArrayList
it throws ConcurrentModificationException
and when we remove 0
from same empty ArrayList
it throws NoSuchElementException
.
Please find the code below :
public class Test {
public static void main(String[] argv) {
ArrayList<Integer> list = new ArrayList<Integer>();
Iterator<Integer> it = list.iterator();
try {
list.remove(-1);
} catch (IndexOutOfBoundsException e) {
}
try {
it.next();// Throwing ConcurrentModificationException
} catch (ConcurrentModificationException e) {
System.err.println("ConcurrentModificationException 1");
} catch (NoSuchElementException e) {
System.err.println("NoSuchElementException 1 ");
}
list = new ArrayList<Integer>();
it = list.iterator();
try {
list.remove(0);
} catch (IndexOutOfBoundsException e) {
}
try {
it.next();// Throwing NoSuchElementException
} catch (NoSuchElementException e) {
System.err.println("NoSuchElementException 2");
} catch (ConcurrentModificationException e) {
System.err.println("ConcurrentModificationException 2 ");
}
}
}
From my understanding NoSuchElementException
is fine but why ConcurrentModificationException
is thrown ?
Upvotes: 3
Views: 91
Reputation: 719739
It is (IMO) a bug, albeit a very minor one.
The code looks like this (in Java 8):
public E remove(int index) {
rangeCheck(index);
modCount++;
E oldValue = elementData(index);
The rangeCheck
call checks that index < size
, but not index >= 0
. Presumably, that is to avoid a redundant check ... because the elementData
call does a non-optimizable array bounds check that deals with the negative index case.
But the catch is that the modCount++
is happening before the array bounds check. Ooops!
A potential fix would be to move the modCount++
after the elementData
call, but this may not be correct. The deal is that modCount
is a volatile
, and therefore reading / writing it has an implicit synchronizing effect for the access to the elementData
that happens next.
Would they fix it if you reported it? Probably not!
Fixing it has the potential to break code that "works" if you do it the way I suggested above. Admittedly the code that you are breaking is incorrect anyway because it is relying on the serendipitous synchronization due to the volatile
. Even so, people whose code broke would scream. (Not least because this would be a difficult breakage to diagnose.)
Alternatively, if you add an explicit size >= 0
check, you most likely make working code measurably slower.
The "bug" only affects code that is incorrect anyway. You should not be calling ArrayList::remove
concurrently with an iteration.
Upvotes: 3
Reputation: 12346
If you check the code for ArrayList. First a range check is performed, then a modification count is added.
rangeCheck(index);
modCount++;
In the range check method, the range check is for positive numbers only.
if (index >= size)
throw new IndexOutOfBoundsException(outOfBoundsMsg(index));
So remove(0)
doesn't add a mod count, but remove(-1)
does. The modCount causes the iterator to throw a ConcurrentModificationException.
Upvotes: 4