Java 8 stream API: Exceptions when modifying Lists

Question

Let's take an ArrayList and fill it with with something simple:

List<String> list = new ArrayList<>();
for (int i = 0; i < 10; i++) {
    list.add(""+i);
}

I will try to remove one member, say named 5, with a different ways of stream API. For this I define the method, that will give me a ConcurentModificationException when using traditional iteration with iterator.

void removeMember(String clientListener) {
    list.remove(clientListener);
}

This code gives me that exception, which I understand:

list.parallelStream()
    .filter(string -> string.equalsIgnoreCase("5"))
    .forEach(string -> removeMember(string));

However, trying just stream(), not parallelStream() gives an null pointer exception (NPE), which is strange for me:

list.stream()
    .filter(string -> string.equalsIgnoreCase("5"))
    .forEach(string -> removeMember(string));

Now change the List type to LinkedList<>. Last code with stream() gives me a ConcurentModificationException, and parallelStream() suddenly works!

So, the questions.

1. Is the internal parallelStream() kitchen (Spliterators and other magic) smart enough to use such element removal for LinkedList? Will it always work?

2. Why was that NPE for ArrayList? Why NPE, not ConcurentModificationException I mean.

Answer 1

The behaviour of your code is essentially undefined (hence the various answers you get). The stream documentation (Non-Intereference section) states:

Unless the stream source is concurrent, modifying a stream's data source during execution of a stream pipeline can cause exceptions, incorrect answers, or nonconformant behavior.

And ArrayList and LinkedList are not concurrent.

You could use a concurrent source but it would make more sense to move away from modifying the source of the stream, for example by using Collection#removeIf:

list.removeIf(string -> string.equalsIgnoreCase("5"));

Answer 2

Adding some debug prints to the pipeline shows the source of the NullPointerException:

list.stream().peek(string -> System.out.println("peek1 " + string)).filter(string -> string.equalsIgnoreCase("5")).peek(string -> System.out.println("peek2 " + string)).forEach(string -> removeMember(string));

This outputs:

peek1 0
peek1 1
peek1 2
peek1 3
peek1 4
peek1 5
peek2 5
peek1 7
peek1 8
peek1 9
peek1 null
Exception in thread "main" java.lang.NullPointerException
    at HelloWorld.lambda$main$1(HelloWorld.java:22)
    at HelloWorld$$Lambda$2/303563356.test(Unknown Source)
    at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:174)
    at java.util.stream.ReferencePipeline$11$1.accept(ReferencePipeline.java:373)
    at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1374)
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:512)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:502)
    at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
    at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:418)
    at HelloWorld.main(HelloWorld.java:22)

When "5" was removed from the List, all the elements from "6" to "9" were shifted one position to the left (i.e. their indices were decremented by 1). The Stream pipeline didn't detect it, so it skipped "6", and when it processed the last position (that originally contained "9"), it encountered null, which resulted in NullPointerException when string.equalsIgnoreCase("5") was evaluated for it.

This is similar to what you'd get in this traditional for loop:

int size = list.size();
for (int i = 0; i < size; i++) {
    String string = list.get(i);
    if (string.equalsIgnoreCase("5"))
        removeMember(string);
}

Only here you'd get IndexOutOfBoundsException instead of NullPointerException, since list.get(i) would fail when i==9. I guess the Stream pipeline works directly on the internal array of the ArrayList, so it doesn't detect that the size of the List has changed.

EDIT:

Following Holger's comment, I changed the code to eliminate the NullPointerException (by changing the filter to filter(string -> "5".equalsIgnoreCase(string))). This indeed produces ConcurrentModificationException:

peek1 0
peek1 1
peek1 2
peek1 3
peek1 4
peek1 5
peek2 5
peek1 7
peek1 8
peek1 9
peek1 null
Exception in thread "main" java.util.ConcurrentModificationException
    at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1380)
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:512)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:502)
    at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
    at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:418)
    at HelloWorld.main(HelloWorld.java:22)

Answer 3

If you want to use streams, instead of modifying the original collection (see immutability with its inherent thread-safety), you should just retrieve a new list without that element:

list.stream().filter(string -> !string.equalsIgnoreCase("5"))
                    .collect(Collectors.toList());

Regarding your other question about parallelStream and if that approach could always work?

No, it will definitely not. The Lists you are using are not built to support concurrent access, sometimes it will appear to work, other times it will fail as you saw or give you "unexpected" results. If you know that a data structure will be accessed by multiple thread always code accordingly.