1

I am currently trying to learn how to properly handle multi-threaded access to Collections, so I wrote the following Java application.

As you can see, I create a synchronized ArrayList which I try to access once from within a Thread and once without.

I iterate over the ArrayList using a for loop. In order to prevent multiple access on the List at the same time, I wrapped the loop into a synchronized block.

public class ThreadTest {

    Collection<Integer> data    = Collections.synchronizedList(new ArrayList<Integer>());
    final int           MAX     = 999;

    /**
     * Default constructor
     */
    public ThreadTest() {
        initData();
        startThread();
        startCollectionWork();
    }

    private int getRandom() {
        Random randomGenerator = new Random();
        return randomGenerator.nextInt(100);
    }

    private void initData() {
        for (int i = 0; i < MAX; i++) {
            data.add(getRandom());
        }
    }

    private void startCollectionWork() {
        System.out.println("\nStarting to work on data outside of thread");
        synchronized (data) {
            System.out.println("\nEntered synchronized block outside of thread");
            for (int value : data) { // ConcurrentModificationException here!
                if (value % 5 == 1) {
                    System.out.println(value);
                    data.remove(value);
                    data.add(value + 1);
                } else {
                    System.out.println("value % 5 = " + value % 5);
                }
            }
        }
        System.out.println("Done working on data outside of thread");
    }

    private void startThread() {
        Thread thread = new Thread() {
            @Override
            public void run() {
                System.out.println("\nStarting to work on data in a new thread");
                synchronized (data) {
                    System.out.println("\nEntered synchronized block in thread");
                    for (int value : data) { // ConcurrentModificationException
                        if (value % 5 == 1) {
                            System.out.println(value);
                            data.remove(value);
                            data.add(value + 1);
                        } else {
                            System.out.println("value % 5 = " + value % 5);
                        }
                    }
                }
                System.out.println("Done working on data in a new thread");
            }
        };
        thread.start();
    }
}

But everytime one of the for loops gets entered, I get a ConcurrentModificationException. This is my console output (which changes with every new run):

Starting to work on data outside of thread

Entered synchronized block outside of thread
51

Starting to work on data in a new thread

Entered synchronized block in thread
value % 5 = 2
value % 5 = 2
value % 5 = 4
value % 5 = 3
value % 5 = 2
value % 5 = 2
value % 5 = 0
21
Exception in thread "main" java.util.ConcurrentModificationException
    at java.util.AbstractList$Itr.checkForComodification(AbstractList.java:372)
    at java.util.AbstractList$Itr.next(AbstractList.java:343)
    at ThreadTest.startCollectionWork(ThreadTest.java:50)
    at ThreadTest.<init>(ThreadTest.java:32)
    at MultiThreadingTest.main(MultiThreadingTest.java:18)
Exception in thread "Thread-1" java.util.ConcurrentModificationException
    at java.util.AbstractList$Itr.checkForComodification(AbstractList.java:372)
    at java.util.AbstractList$Itr.next(AbstractList.java:343)
    at ThreadTest$1.run(ThreadTest.java:70)

What's wrong?

Ps: Please don't just post links to multi-threaded how-to's since I've already read enough about that. I am just curious why my application doesn't run as I want it to.

Update: I replaced the for(x : y) syntax with an explicit Iterator and a while loop. The problem remains though..

synchronized(data){
        Iterator<Integer> i = data.iterator();
        while (i.hasNext()) {
        int value = i.next(); // ConcurrentModificationException here!
        if (value % 5 == 1) {
            System.out.println(value);
            i.remove();
            data.add(value + 1);
        } else {
            System.out.println("value % 5 = " + value % 5);
        }
    }
}
Improve this question

7 Answers 7

Reset to default
4

Once you iterate over a collection, you have a contract between the iterating block of code and the collection as it exists at that moment in time. The contract basically states that you'll get each item in the collection once, in the order of the iteration.

The problem is that if you modify the Collection while something is iterating, you cannot maintain that contract. Deletions in a collection will remove the element from the collection, and that element might be required to be present for the initial iteration to satisfy the contract. Insertions in a Collection will likewise present issues if the element might be detected by the iteration that started prior to the element existing in the collection.

While it is easier to break this contract with multiple threads, you can break the contract with a single thread (if you choose to do so).

How this is typically implemented is the collection contains a "revision number", and prior to the iterator grabbing the "next" element in the collection, it checks to see if the collection's revision number is still the same as it was when the iterator started. This is just one way of implementing it, there are others.

So, if you want to iterate over something that you might want to change, an appropriate technique is to make a copy of the collection and iterate over that copy. That way you can modify the original collection and yet not alter the count, position, and presence of the items you were planning to process. Yes, there are other techniques, but conceptually they all fall into the "protect the copy you're iterating across while changing something else that the iterator doesn't access".

Improve this answer
Sign up to request clarification or add additional context in comments.

8 Comments

Great answer! Thanks a lot. Iterating over a copy and modifying the original collection at the same time solved the issue!
There is a corner case for removal. Some iterators implement the iterator.remove() operation, which allows an iterator to (only once per element) remove the current element. Since the iterator knows it has already presented the current element, it won't worry about the collection modification. One point to be aware of though, is that iterator.remove() is an optional operation for iterators. In other words, some iterators (for particular collections) just throw a OperationNotSupported exception every time you call iterator.remove().
Also , calling one iterator's remove method may still lead another open iterator to throw ConcurrentModificationException, so the remove method is not completely safe against the exception.
@Bhaskar, good point. Copying is the "safest" way to go. That's why copy-on-write is so popular a paradigm amongst the concurrent people.
@Edwin Buck So, if I created a copy of the original data and iterate over the copy, shouldn't I be safe if I call originalList.remove(...) instead of iterator.remove(...)?
|
2

The ConcurrentModificationException appears, because you're modifing the list while iterating it... it has nothing to do with multiple threads in this case...

for (int value : data) { // ConcurrentModificationException here!
                if (value % 5 == 1) {
                    System.out.println(value);
                    data.remove(value); // you cannot do this
                    data.add(value + 1); // or that
                } else {
                    System.out.println("value % 5 = " + value % 5);
                }
Improve this answer

2 Comments

Uh, so if I iterate through a collection and wanted to remove a specific object (when a condition x equals to true), I cannot do that? How could I work around that?
Personally I save the items I want to add/remove in another list and save/remove them after the iteration finished
2

The enhanced for loop 

for (int value : data) {

uses a Java Iterator under the covers. Iterators are fail-fast, so if the underlying Collection gets modified (i.e. by removing an element) while the Iterator is active you get the Exception. Your code here causes such a change to the underlying Collection:

data.remove(value);
data.add(value + 1);

Change your code to use java.util.Iterator explicitly and use its remove method. If you need to add elements to the Collection while iterating you may want to look at a suitable data structure from java.util.concurrent package e.g. BlockingQueue, where you can call its take method which will block until there is data present; but new Objects can be added via the offer method (very simplistic overview - Google for more)

Improve this answer

1 Comment

I replaced the for (x : y) syntax with a while loop and calls on the explicit Iterator (see bottom of the updated question) but the problem still remains. So, I guess it doesn't matter HOW I retrieve the iterator, or not?
1

The problem is not because of multithreading, you made if safe. But only because you modified collection while iterating it.

Improve this answer

Comments

1

You are only allowed to remove items from the list through an iterator if you are iterating over the collection, so you can get ConcurrentModificationException with only one thread.

Updated reply after updated question: You aren't allowed to add elements to the list while you are iterating.

Improve this answer

2 Comments

I updated my code (see bottom of my original question) and only remove items through the iterator. Still, the Exception gets thrown..
@valmar, you aren't allowed to add elements to the list while iterating.
0

As has been pointed out, you are modifying the Collection while iterating over it, which causes your problem.

Change data to a List and then use a regular for loop to step over it. Then you will no longer have an Iterator to deal with, thus eliminating your problem.

List<Integer> data =...

for (int i=0; i<data.size(); i++) {
        int value = data.get(i);

        if (value % 5 == 1) {
            System.out.println(value);
            i.remove();
            data.add(value + 1);
        } else {
            System.out.println("value % 5 = " + value % 5);
        }
}
Improve this answer

Comments

0

ConcurrentModificationException is not eliminated by using a synchronized block around the collection on which iteration is done. The exception occurs on the following sequence of steps :

  1. Obtain an iterator from a collecion ( by calling its iterator method or by the for loop construct ).
  2. Begin iterating ( by calling next() or in the for loop )
  3. Have the collection modified ( by any means other than the iterator's methods ) ( either in the same thread or a different one : this is what is happening in your code ). Note that this modification may happen in a thread-safe manner ie sequentially or one after another - it does not matter- it will still lead to CME )
  4. Continue iteration using the same iterator obtained earlier ( before modification in step 3 )

In order to avoid getting the exception , you must make sure that you do not modify the collection after you start your for loop in either of the threads, till the loop is finished.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.