CODEWITHSUNDEEP
javaforeachjava-8
Shashi Shankar
Shashi Shankar

Reputation: 809

Java forEach lambda throws concurrentModificationException

What could be the reason that below method throws ConcurrentModificationException?

static Set<String> setOfAllocAccountsIn(final @NotNull Execution execution) {
        final Set<String> allocAccounts = new HashSet<>();
        execution.legs().forEach(leg -> leg.allocs().forEach(alloc -> {
            if (alloc.account() != null) {
                allocAccounts.add(alloc.account());
            }
        }));
        return allocAccounts;
    }

Stack Trace:

"java.util.ConcurrentModificationException:
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1382)
    at com.client.stp.util.DealsBuilderUtil.setOfAllocAccountsIn(DealsBuilderUtil.java:146)
    at com.client.stp.builder.GridDealsObjectBuilder.build(GridDealsObjectBuilder.java:47)
    at com.client.stp.processor.DealToPDXMLTransformer.transform(DealToPDXMLTransformer.java:29)
    at com.client.stp.processor.WorkPartitionerEngine.lambda$onEventSubmit$0(WorkPartitionerEngine.java:40)
    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:514)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
    at java.base/java.lang.Thread.run(Thread.java:844)

With simple for-loop it works fine. The method is called by multiple threads with its own copy of execution object.

Solution which worked with simple for loop:

 static Set<String> setOfAllocAccountsIn(final @NotNull Execution execution) {
        final Set<String> allocAccounts = new HashSet<>();
        for (final ExecutionLeg executionLeg : execution.legs()) {
            for (final ExecutionAlloc executionAlloc : executionLeg.allocs()) {
                if (executionAlloc.account() != null) {
                    allocAccounts.add(executionAlloc.account());
                }
            }
        }
        return allocAccounts;
    }

I feel that it has something to do with static method and its local variable accessed by multiple threads but per theory that will be thread local variable and are not shared. Give me some time to put down some simple example.

Upvotes: 0

Views: 752

Answers (1)

Youcef LAIDANI
Youcef LAIDANI

Reputation: 59960

Your logic can be like this :

return execution.legs().stream()
            .flatMap(leg -> leg.allocs().stream())
            .map(executionAlloc -> executionAlloc.account())
            .filter(Objects::nonNull)
            .collect(Collectors.toSet());
  • You are using forEach inside forEach (Here I replace it with flatMap)
  • then you check for each element if alloc.account() is null or not (I replace it with filter)
  • then if the condition is correct you add it to the Set (I replace it with a collect)

Upvotes: 2

Related Questions