ConcurrentModificationException - Tried Synchronizing, ListIterator, etc.

[DougD720]

Hello all,

I rarely come across a Java problem I can't figure out myself but this one's got me stumped. Basically I have a program that needs to run an update everyday. In order to maximize efficiency I made this multi-threaded and put a cap on the number of concurrent threads that can run. So the update function basically creates a List<Thread> of threads and then creates another list of Active Threads (also a List<Thread>). Both of which are ArrayLists. Then it runs through a while loop (below) and starts threads when the number of active threads is below a certain number and then scans the list to remove any threads which are finished. The threads themselves have a few boolean statuses (started, finished, etc.) and synchronized functions to access those variables. I keep getting ConcurrentModificationExceptions (the line is noted in the below code) I imagine because the UpdateThread objects are changing themselves (changing some boolean variables when they're finished). I tried throwing a synchronized block around the loop code (even though I feared that would hurt the performance), I tried a ListIterator, etc. but it still didn't work. The UpdateThread objects themselves just connect to a website, download some data, parse it and then upload it to a local database, but I don't think that's what's causing the issues, just FYI.

If anyone has any ideas I'd greatly appreciate it, the sooner I get this working the better, it's the last component of the program.

        //Execute the threads, removed completed ones, until all threads are done
        List<UpdateThread> activeThreads = new ArrayList<UpdateThread>();
        while(!threads.isEmpty() || !activeThreads.isEmpty())
        {
            //Check if we can add more threads
            while(activeThreads.size() < UpdateThread.MAX_CONCURRENT_THREADS && threads.size() > 0)
            {
                //Add next thread to active threads until full
                activeThreads.add(threads.remove(0));
            }
            //Update the active threads list, start any unstarted, remove any finished
            for(UpdateThread ut : activeThreads)   // *** ERROR is thrown HERE
            {
                if(ut.isFinished())
                {
                    //Remove any finished
                    if(ut.success())
                    {
                        updatedSuccessfully++;
                    }
                    activeThreads.remove(ut);
                    updated++;
                }
                else if(ut.isRunning() == false)
                {
                    //Start any unstarted
                    ut.start();
                }
                else{
                    //Do nothing, running but not done
 
                }
            }
            //Sleep
            try{
                Thread.sleep(UpdateThread.UPDATE_THREAD_CHECK_SLEEP);
            }catch(Exception e)
            {
                //Do Nothing
            }
        }

Thanks!

--- Update ---

Figured it out, I'm an idiot. Just had to remove it using the iterator:

            //Update the active threads list, start any unstarted, remove any finished
            Iterator<UpdateThread> iter = activeThreads.iterator();
            while(iter.hasNext())
            {
                UpdateThread ut = iter.next();
                if(ut.isFinished())
                {
                    //Remove any finished
                    if(ut.success())
                    {
                        updatedSuccessfully++;
                    }
                    iter.remove();
                    updated++;
                }
                else if(ut.isRunning() == false)
                {
                    //Start any unstarted
                    ut.start();
                }
                else{
                    //Do nothing, running but not done
 
                }
            }
            //Sleep
            try{
                Thread.sleep(UpdateThread.UPDATE_THREAD_CHECK_SLEEP);
            }catch(Exception e)
            {
                //Do Nothing
            }

In case anyone else has this issue, here you go. Works like a charm.

[jps]

Thank you for posting back with the solution to the problem you were having.