Getting concurrent modification exception while using iterator in multithreaded program

Question

My code is:

class Processor implements Runnable {

    private int id;
    private Integer interaction;
    private Set<Integer> subset;
    private volatile static AtomicBoolean notRemoved = new AtomicBoolean(true);

    public Object<E> dcp;
    public Iterator<Integer> iterator;



    public Processor(int id, Integer interaction, Set<Integer> subset, Object<E> dcp, Iterator<Integer> iterator) {
        this.id = id;
        this.interaction = interaction;
        this.subset= subset;
        this.dcp = dcp;
        this.iterator = iterator;
    }

    public void run() {
        while (Processor.notRemoved.get()){
            System.out.println("Starting: " + this.subset);
            if (this.dcp.PA.contains(this.interaction)){
                this.subset.add(this.interaction);
                this.dcp.increaseScore(this.subset);
                if (!this.subset.contains(this.interaction) && Processor.notRemoved.get()){
                    Processor.notRemoved.set(false);
                    iterator.remove();
                }
            }

            System.out.println("Completed: " + this.id);
        }
    }
}


public class ConcurrentApp {

    public void multiThreadProgram (Object<E> dcp, int threads) {

        ExecutorService executor = Executors.newFixedThreadPool(threads);

        int i =1;
        while ((dcp.PA.size() > i) && (i <= dcp.R)){
            for (Iterator<Integer> iterator = dcp.PA.iterator(); iterator.hasNext();){
                Integer interaction = iterator.next();
                ArrayList<Integer> removed = new ArrayList<Integer>(dcp.PA);
                removed.remove(interaction);
                ArrayList<Set<Integer>> subsets = dcp.getSubsets(removed, i);
                for (int j = 0; j< subsets.size(); j++){
                    try {
                        executor.submit(new Processor(j, interaction, subsets.get(j), dcp, iterator));
                    } catch (RejectedExecutionException e){
                        System.out.println("Task was rejected");
                    }   
                }
            }
            System.out.println("All tasks completed");
            i++;
        }
        executor.shutdown();
        System.out.println("All tasks submitted");
        try {
            executor.awaitTermination(1, TimeUnit.DAYS);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }
}

I'm getting java.util.ConcurrentModificationException at java.util.ArrayList$Itr.checkForComodification(Unknown Source) at java.util.ArrayList$Itr.next(Unknown Source)....

However, I thought that by using the iterator, I could avoid this problem of iterating over my collection while modifying my collection at the same time. I didn't have this problem in my non-multithreaded implementation of my program so I'm assuming this has to do with multiple threads doing something with iterator. Does anyone have any ideas?

How did you ever get `Object` to compile? `Object` is not a generic type. — Lew Bloch, Mar 16 '17 at 23:15
@LewBloch Sorry about that. I had to remove that portion of the code and replaced with Object because I'm not supposed to share that portion of the code according to my supervisor. — g00glechr0me, Mar 16 '17 at 23:17
Pick a name that isn't a, or in this case, the standard Java name for a type. Also, when you leave out significant sections of code, you risk omitting code where the problem lies. — Lew Bloch, Mar 17 '17 at 00:48
You don't need to declare a `AtomicBoolean` as `volatile`. I dare say it should be `final`. — Lew Bloch, Mar 17 '17 at 00:50
@Lew Bloch i was told in another post that AtomicBoolean is already volatile and I don't need the volatile keyword. I got the code to run but now for some reason, my program never enters the last if condition in the run function of processor and thus no value is ever deleted from the iterator. I changed PA to a CopyOnWriteList but still no luck, any ideas? — g00glechr0me, Mar 17 '17 at 00:53
`AtomicBoolean` is not per se volatile but the effect is similar. — Lew Bloch, Mar 17 '17 at 00:58

score 2 · Accepted Answer · edited May 23 '17 at 11:53

2

Java's ArrayList isn't thread safe, regardless of whether you use an iterator.

From the Javadoc:

If multiple threads access an ArrayList instance concurrently, and at least one of the threads modifies the list structurally, it must be synchronized externally.

...

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

Take a look at this question for a discussion of thread-safe alternatives. Basically, you'll need to either use locks (such as synchronized blocks), or a suitable thread-safe list.

edited May 23 '17 at 11:53

Community

1
1

answered Mar 16 '17 at 21:23

Malt

28,965
9
65
105

Do you have any suggestions on how I can implement a thread-safe version of this? – g00glechr0me Mar 16 '17 at 21:24
How about using ConcurrentHashMap? You can also get a set from your concurrent HashMap. – yogidilip Mar 16 '17 at 21:26
http://stackoverflow.com/questions/6720396/different-types-of-thread-safe-sets-in-java – yogidilip Mar 16 '17 at 21:28
@Chandra_Rathnam I've added that to the answer. – Malt Mar 16 '17 at 21:28
Appreciate the help, I'll look into both of your responses! – g00glechr0me Mar 16 '17 at 21:41

score 1 · Answer 2 · answered Mar 16 '17 at 21:28

I thought that by using the iterator, I could avoid this problem of iterating over my collection while modifying my collection at the same time.

This is true about ListIterator<T>s, not simply Iterator<T>s. In order to avoid ConcurrentModificationException on removal, several things must be true:

ListIterator<T> must support remove() - this operation is optional
Your code must remove the item by calling list iterator's remove() - removing directly from the list does not work
There must be no concurrent removals through other iterators - ListIterator<T> takes care of removals through the same iterator object; they do not help when your code removes items concurrently from different threads.

Getting concurrent modification exception while using iterator in multithreaded program

2 Answers2