0

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.

7
  • Given that you don't show the code inside the foreach loop, no one can tell. Also, is your list modified outside of the runnable?
    – fge
    Commented Jul 23, 2013 at 5:00
  • 1
    I will bet a beer that either the caller, or your loop is modifying batch.
    – user949300
    Commented Jul 23, 2013 at 5:02
  • The processing inside the loop only uses record, and makes no reference to batch.
    – ddnm
    Commented Jul 23, 2013 at 5:19
  • Use a List iterator instead of directly iterating over the list and modifying it.
    – Rakesh
    Commented Jul 23, 2013 at 5:27
  • Your UPDATEs are proof that caller, or somebody, is modifying the list. p.s. try making your instance var batch final.
    – user949300
    Commented Jul 23, 2013 at 5:34

7 Answers 7

2

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.

12
  • I had assumed it was thread safe if each new thread is a seperate instance. Could you suggest a modification that would be thread-safe?
    – ddnm
    Commented Jul 23, 2013 at 4:40
  • I'm missing something. His RunnableTask has a reference to the List. Which isn't changing after construction. There is no "the list reference", there are n separate references. As I understand his code. Note: would be better if batch was declared final.
    – user949300
    Commented Jul 23, 2013 at 4:54
  • @howlerdingdong. You can take a look at java.util.concurrent.CopyOnWriteArrayList
    – Rohit Jain
    Commented Jul 23, 2013 at 4:56
  • @user949300. You can create multiple threads passing the same reference of RunnableTask. Try doing this.
    – Rohit Jain
    Commented Jul 23, 2013 at 4:56
  • Assuming that run() doesn't modify batch it should work. Though this late at night I may be missing something. My bet is that his loop in run() is indirectly changing something.
    – user949300
    Commented Jul 23, 2013 at 5:01
0

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

0

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.

2
  • How would changing the batch size affect concurrency in any way?
    – ddnm
    Commented Jul 23, 2013 at 5:33
  • no you missd the point, It is creating a new whole new list Object Commented Jul 23, 2013 at 6:11
0

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}
     }
   }
1
  • Using synchronized on the for loop still gives me exception because the change to batch is happening when new thread instantiations modify batch at the class level.
    – ddnm
    Commented Jul 23, 2013 at 5:32
0

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.

3
  • Performance is an issue unfortunately, I'm processing batches in the hundreds of thousands.
    – ddnm
    Commented Jul 23, 2013 at 4:51
  • Then use ReentrantLock. It does not lock while reading(what you are apparently doing in processing) it does a volatile read.It will only lock for write. Commented Jul 23, 2013 at 4:55
  • Both reentrantlock and synchronized actually still give me the same exception, given again that the modification isn't occurring from the for loop but from the reinstantiation of batch with every new thread.
    – ddnm
    Commented Jul 23, 2013 at 5:26
0

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

0

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.

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.