CODEWITHSUNDEEP
javamultithreadinglistconcurrency
ddnm
ddnm

Reputation: 187

java.util.ConcurrentModificationException: Unexpected List modification while multithreading?

I'm using multithreading to process a List of Strings in batches, however I'm getting this error when the Runnable task is iterating over the List to process each String.

For example the code roughly follows this structure:

public class RunnableTask implements Runnable {

private List<String> batch;

    RunnableTask(List<String> batch){
        this.batch = batch;
    }


    @Override
    public void run() {

        for(String record : batch){

            entry = record.split(",");
            m = regex.matcher(entry[7]);

            if (m.find() && m.group(3) != null){
                currentKey = m.group(3).trim();
                currentValue = Integer.parseInt(entry[4]);

                if ( resultMap.get(currentKey) == null ){
                    resultMap.put(currentKey, currentValue);
                } else {
                    resultMap.put(currentKey, resultMap.get(currentKey) + currentValue);
            }   
        }

    }
} 
}

Where the thread that is passing these batches for processing never modifies "batch" and NO CHANGES to batch are made inside the for loop. I understand that this exception ConcurrentModificationException is due to modifying the List during iteration but as far as I can tell that isn't happening. Is there something I'm missing?

Any help is appreciated,

Thankyou!

UPDATE1: It seems instance-variables aren't thread safe. I attempted to use CopyOnWriteArrayList in place of the ArrayList but I received inconsistent results - suggesting that the full iteration doesn't complete before the list is modified in some way and not every element is being processed.

UPDATE2: Locking on the loop with sychronized and/or a reentrantlock both still give the same exception.

I need a way to pass Lists to Runnable tasks and iterate over those lists without new threads causing concurrency issues with that list.

Upvotes: 0

Views: 772

Answers (7)

Adrian Shum
Adrian Shum

Reputation: 40036

Obviously someone else is changing the content of the list, which is out of picture of the code you mentioned. (If you are sure that the ConcurrentModificationException is complaining for the batch list, but not resultMap, and you are actually showing all code in RunnableTask)

Try to search in your code, for places that is updating the content of the list, check if it is possible concurrently with your RunnableTask.

Simply synchronizing in the RunnableTask is not going to help, you need to synchronize all access to the list, which is obviously happening somewhere else.

If performance is an issue to you so that you cannot synchronize on the batch list (which prohibit multiple RunnableTask to execute concurrently), consider making use of ReaderWriterLock: RunnableTask acquires read lock, while the list update logic acquire the write lock.

Upvotes: 0

user949300
user949300

Reputation: 15729

Your followups indicate that you are trying to reuse the same List multiple times. Your caller must create a new List for each Runnable.

Upvotes: 0

Rohit Jain
Rohit Jain

Reputation: 213253

I understand that this exception ConcurrentModificationException is due to modifying the List during iteration but as far as I can tell that isn't happening

Ok, consider what happens when you create a new thread, passing a reference to RunnableTask instance, initialized with a different list as constructor parameter? You just changed the list reference to point to different list. And consider what happens when at the same time, a different thread inside the run() method, is changing the list, at any point. This will at some point of time, throw ConcurrentModificationException.

Instance Variables are not Thread-Safe.

Upvotes: 2

Aniket Thakur
Aniket Thakur

Reputation: 68935

yes you are getting ConcurrentModificationException because your List is getting modified during iteration. If performance is not a critical issue I suggest use synchronization. public class RunnableTask implements Runnable {

private List<String> batch = new ArrayList<String>();

RunnableTask(List<String> batch){
    this.batch = batch;
}


public void run() {
    synchronized (batch) {
        for(String record : batch){//do processing with record}
        }
    }

}
}

or even better use ReentrantLock.

Upvotes: 0

Debobroto Das
Debobroto Das

Reputation: 862

you need to lock the list before accessing its elements. because List is not thread safe. Try this

   public void run() {
     synchronizd(batch){
         for(String record : batch){//do processing with record}
     }
   }

Upvotes: 0

Subhrajyoti Majumder
Subhrajyoti Majumder

Reputation: 41200

Problem is due to multiple thread concurrently modifying the the source List structure. What I would suggest you should devide the source list to new sublist(according to size) and pass that list to threads.

Say your source List have 100 elements. and you are running 5 concurrent thread.

int index = 0;
List<TObject> tempList = new ArrayList<>();
for(TObject obj:srcList){
    if(i==(srcList.size()/numberOfthread)){
      RunnableTask task = new RunnableTask(tempList);
      tempList = new ArrayList<>();
    }else 
       tempList.add(obj);
}

In this case your original list would not be modified.

Upvotes: 0

morgano
morgano

Reputation: 17422

Try this in your code:

public void run() {
    for(String record : new ArrayList(batch)){
        //do processing with record
    }
}

There is a sort of problem with all your threads processing the list (is the list modified during the process?) but is difficult to tell with the code you're providing

Upvotes: 0

Related Questions