3

I got this code Snippet which is working fine.

import java.util.ConcurrentModificationException;
import java.util.*;

ArrayList<Object> s = new ArrayList<>();

s.add(null);
s.add("test");

try {
    System.out.println(s + "\n");
    for (Object t : s) {
        System.out.println(t);

        if (t == null || t.toString().isEmpty()) {
            s.remove(t);
            System.out.println("ObjectRemoved = " + t + "\n");
        }

    }

    System.out.println(s + "\n");
} catch (ConcurrentModificationException e) {
    System.out.println(e);
} catch (Exception e) {
    System.out.println(e);
}

But after changing

s.add(null);
s.add("test");

to

s.add("test");
s.add(null);

The code throws a ConcurrentModificationException

So my question is why can I remove null if it's the first object in the list but not if it's the second one?

ernest_k
  • 44,416
  • 5
  • 53
  • 99
  • Because you are modifiying the list while iterating other than via the iterator. You need to use an explicit iterator and use `Iterator.remove()`. – user207421 Aug 12 '20 at 08:16

4 Answers4

4

The first thing to understand is that the code is invalid, as it structurally modifies the list while iterating over it and this is not permitted (NB: this is a slight simplification, as there are pemissible ways to modify a list, but this is not one of them).

The second thing to understand is that ConcurrentModificationException works on a best-effort basis:

Fail-fast iterators throw ConcurrentModificationException on a best-effort basis. Therefore, it would be wrong to write a program that depended on this exception for its correctness: the fail-fast behavior of iterators should be used only to detect bugs.

And so invalid code may or may not raise ConcurrentModificationException -- it's really up to the Java runtime and could depend on the platform, version etc.

For example, when I try [test, null, null] locally, I get no exception but also an invalid result, [test, null]. This is a different behaviour to the two behaviours that you observed.

There are several ways to fix your code, the most common of which is probably to use Iterator.remove().

NPE
  • 486,780
  • 108
  • 951
  • 1,012
  • So what you´re saying is you should never modify a List while iterating it. So i guess the best thing to do is, making a new List without the null values, and after iterating move the values to the other list, right? – Lukas Forman Aug 12 '20 at 08:19
  • 1
    @LukasForman: The way to think about it is that a structural modification invalidates all iterators. So iterating by index is OK (as long as you account for how deleting an element affects the indices of the remaining elements). It is also OK to remove an element using `Iterator.remove()`. – NPE Aug 12 '20 at 08:23
  • Okay I think I got it. Thanks for your help. – Lukas Forman Aug 12 '20 at 08:25
  • Ah now it took me so long to type my answer, I didn't realize there's already a valid one. Anyway, I think I added some detail about the relationship of for-each and `Iterable`, so I'll leave the answer for anyone who wants to read into that. +1 – bkis Aug 12 '20 at 08:35
  • Your ['test', null, null] result is the same as the [null, 'test'] version. The null is removed and the loop stops, never actually iterating over 'test' – matt Aug 12 '20 at 08:54
1

Well, in a best case scenario, the ConcurrentModificationException should be thrown in any of the two cases. But it isn't (see the quote from the docs below).

If you use the for-each loop to iterate over an Iterable, the Iterator returned by the iterator() method of the data structure will be used internally (the for-each loop is just syntactic sugar).

Now you shouldn't (structurally) modify an Iterable that is being iterated after this Iterator instance was created (it's illegal unless you use the Iterator's remove() method). This is what a concurrent modification is: There are two different perspectives on the same data structure. If it's modified from one perspective (list.remove(object)), the other perspective (the Iterator) won't be aware of this.

It is not about the element being null. The same happens if you change the code to remove the string:

ArrayList<Object> s = new ArrayList<>();

s.add("test");
s.add(null);

try {
    System.out.println(s + "\n");
    for (Object t : s) {
        System.out.println(t);

        if (t != null && t.equals("test")) {
            s.remove(t);
            System.out.println("ObjectRemoved = " + t + "\n");
        }

    }

    System.out.println(s + "\n");
} catch (ConcurrentModificationException e) {
    System.out.println(e);
} catch (Exception e) {
    System.out.println(e);
}

Now the reason this behaviour differs in some scenarios is simply the following (from the Java SE 11 Docs for ArrayList):

The iterators returned by this class's iterator and listIterator methods are fail-fast: if the list is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove or add methods, the iterator will throw a ConcurrentModificationException. Thus, in the face of concurrent modification, the iterator fails quickly and cleanly, rather than risking arbitrary, non-deterministic behavior at an undetermined time in the future.

Note that the fail-fast behavior of an iterator cannot be guaranteed as it is, generally speaking, impossible to make any hard guarantees in the presence of unsynchronized concurrent modification. Fail-fast iterators throw ConcurrentModificationException on a best-effort basis. Therefore, it would be wrong to write a program that depended on this exception for its correctness: the fail-fast behavior of iterators should be used only to detect bugs.

bkis
  • 2,530
  • 1
  • 17
  • 31
0

This exception occurred because of remove element while iterating on a list. To solve this problem you can use iterator like this:

for (Iterator iterator = s.iterator(); iterator.hasNext(); ) {
    Object eachObject = iterator.next();
    if (eachObject == null || eachObject.toString().isEmpty()) {
        iterator.remove();
        System.out.println("ObjectRemoved = " + eachObject + "\n");
    }
}
Omid
  • 314
  • 1
  • 13
0

The odd behavior happens because of the way the iterator is implemented. The for each loop is going to use the ArrayList.iterator() to iterate over the collection.

Iterator<Object> obj = s.iterator();

while(obj.hasNext()){
    Object t = obj.next();
    // the rest of the code you have.
}

In the first case that works your output is.

[null, test]
null
ObjectRemoved = null
[test]

This means the last iteration was skipped. From the ArrayList source code we can see why this is.

public boolean hasNext() {
    return cursor != size;
} 

On your first iteration next is called and the cursor is updated to 1. Then the null is removed. On the subsequent call to hasNext the cursor equals the size and the loop stops. Missing the last iteration, but not throwing a CME

Now, in your second case. The null is the last item in the list, when the loop goes to start again, hasNext is called and it returns true because the cursor is greater than the size of the list! Then the CME occurs when Iterator.next is called.

matt
  • 10,892
  • 3
  • 22
  • 34