ConcurrentModificationException in List
In the following code I am getting a ConcurrentModificationException that I can't seem to rectify.
I have tried
1. sync'*** the 2 public methods
2. sync'*** within the methods (currently shown in code) this was most often recommended in forums that I have read on this problem.
3. sync'*** on the current object ("this", as shown in the code) and sync'*** on the list object itself.
4. Using Collections.synchronizedList(new Vector ...)
After chasing the problem I am pretty sure that the .add method is being called while the Iteraor loop is running, but shouldn't sync'*** fix that? The list is private and is not accessed from any other class outside the current one shown. Is there something wrong with my sync'***?
Any recommendations will be appreciated.
Thanks,
sseric
Error:
Exception in thread "Thread-2" java.util.ConcurrentModificationException
at java.util.AbstractList$Itr.checkForComodification(AbstractList.java:449)
at java.util.AbstractList$Itr.next(AbstractList.java:420)
at genetviz.utils.BroadcastGraphAction.broadcast(BroadcastGraphAction.java:80)
at genetviz.utils.Graph.init(Graph.java:106)
at genetviz.io.FileOpener$FileLoader.run(FileOpener.java:312)
at java.lang.Thread.run(Thread.java:595)
Code:
import java.util.*;
import javax.swing.event.ChangeEvent;
import java.awt.Component;
import java.awt.Dimension;
public abstract class BroadcastGraphAction extends GraphRenderer
implements GraphListener {
public abstract void setGraph(Graph graph);
private Vector<GraphListener> graphListenerList = new Vector<GraphListener>();
public void addGraphListener(GraphListener gl)
{
synchronized(this)
{
graphListenerList.add(gl);
}
}
public void broadcast(Graph gr)
{
if(gr == null) return;
synchronized(this)
{
for (Iterator iter = graphListenerList.iterator(); iter.hasNext();)
{
Object obj = iter.next();
if (obj instanceof GraphListener) ((GraphListener)obj).setGraph(gr);
}
}
}
}
[2213 byte] By [
sserica] at [2007-10-2 10:06:07]
when posting code, remember to use code tags...
anyhow, the exception is thrown when, while iterating the list, the list is changed (either by the thread iterating over the list, either by any other thread)...
except operations with the iterator, you shouldnt add / remove items on the list while iterating over it...
for (Iterator iter = graphListenerList.iterator(); iter.hasNext();)
{
Object obj = iter.next();
if (obj instanceof GraphListener) ((GraphListener)obj).setGraph(gr);
}
i believe the setGraph method is adding the Graph to the list (tough, because of its abstractness i cant really tell)...
sync'inc will only prevent acess by other threads...
So now "i n g" is a naughty word? Crikey!
> So now "i n g" is a naughty word? Crikey!See the following thread, starting at Reply 4: http://forum.java.sun.com/thread.jspa?threadID=700038
Thanks Monica. I guess we're headed towards making everything a naughty word, so eventually maybe all questions will look like this:***** ***** **** ** ***** ***** *******?That'll make replying to them easier, as the only reply would be:****?
takeshi10,
please see below for response,
> when posting code, remember to use code tags...
thank you. I'm new to the forums
>except operations with the iterator, you shouldnt add / remove items on the list while iterating over it...
yep, that's the problem.
>i believe the setGraph method is adding the Graph to the list (tough, because of its abstractness i cant really tell)...
no. the "Graph" is independent of the "GraphListener" and is not being added to the graphListenerList. The problem appears to be that .add is being called after .iterator is called on the graphListenerList. This is what I need to prevent through synchronization, or other techniques.
>sync'inc will only prevent acess by other threads...
right, which is why I used it in my code. So one of 2 things is happening then,
1. my synchronization is correct and it is not another thread causing the exception
2. Something is wrong with my synchronization
Though I'm not familiar with this, I tried something on the first note. I created a thread in the current class keeping it alive with a while loop and Thread.sleep, and I tried locking on it to ensure that an independent thread controlled the locking, for example,
public void addGraphListener(GraphListener gl)
{
if(lock==null)
{ lock = new Thread(new LockRunner());}
synchronized(lock)
{
graphListenerList.add(gl);
}
}
private class LockRunner extends Object implements Runnable
{
public void run()
{
try {
while(true) Thread.sleep(1000);
} catch (InterruptedException e) {
System.out.println("BroadcastGraphAction sleep exception");
}
}
}
public void broadcast(Graph gr)
{
if(gr == null) return;
if(lock==null)
{
lock = new Thread(new LockRunner());
}
synchronized(lock)
{
for (Iterator iter = graphListenerList.iterator(); iter.hasNext();)
{
Object obj = iter.next();
if (obj instanceof GraphListener) ((GraphListener)obj).setGraph(gr);
}
}
}
This did not work either. Unfortunately, I am only marginally experienced with threads and thread locking so I'm not quite sure I'm getting this right (well, obvioudly I'm not getting this right). Other suggestions?
sseric
just an FYI, you dont need the synchronizedList over a Vector (already threadsafe), but you might want to use a synchronizedList out of an ArrayList (it ill give you better concurrency overral)...
but that has nothing to do with your problem...
synchronizing all the methods that somehow (directly or indirectly) acess your List will solve the problem of multithreading concurrency messing up your list (but check every method)...
but as i told you before, prolly somewhere in the setGraph() your calling, say, the addGraphListener and the exception is being thrown...
post the setGraph () method here and well take a look at it...
id suggest copy-on-write solution, but it may render the lists out of sync...
takeshi10,
You are quite right. After some exploration of the setGraph I found that the work I had been doing in the past week created a circular reference. That is, while iterating on thte listeners and calling setGraph, one of my MANY setGraph methods goes through many calls which eventually tries to add another listener to the current class. Thus, while iterating on the listeners the add method was being called. I will not post the process because it's too convoluted and I think you have helped me work through the problem (honestly, I thought I was a better debugger than this. It seems so obvious now where the problem is coming from).
I think I can change the architecture of this one method I created last week and work this out, but I am still confused about something. I always knew that the add method was being called while the listenerlList was being iterated on. What I discovered this morning (with your help) was who the offending caller is. But, regardless of who is calling the add method and creating the exception, shouldn't sychronization protect against that?
> takeshi10,
>
> You are quite right. After some exploration of the
> setGraph I found that the work I had been doing in
> the past week created a circular reference. That is,
> while iterating on thte listeners and calling
> setGraph, one of my MANY setGraph methods goes
> through many calls which eventually tries to add
> another listener to the current class. Thus, while
> iterating on the listeners the add method was being
> called. I will not post the process because it's too
> convoluted and I think you have helped me work
> through the problem (honestly, I thought I was a
> better debugger than this. It seems so obvious now
> where the problem is coming from).
>
> I think I can change the architecture of this one
> method I created last week and work this out, but I
> am still confused about something. I always knew that
> the add method was being called while the
> listenerlList was being iterated on. What I
> discovered this morning (with your help) was who the
> offending caller is. But, regardless of who is
> calling the add method and creating the exception,
> shouldn't sychronization protect against that?
hello again!
the quick answer is no... in your specific case, because of reentracy of the monitors (and thank god for that), the same thread could always modify the list while iterating through it...
in a more generic case, no too...
i suppose theres no easy way of telling the list: im finished iterating... now you can add or delete things out of the list safely and nothing would guarantee that the user (us, who are using the List) would actually call it everytime...
and would prevent safe concurrent read-only access over the list...
and i bet other guys will give you better reasons also...
takeshi10,>the same thread could always modify the list while iterating through it...Ok, that's what I didn't get and needed to know. I threw some Duke Dollars your way. Thanks for your help, I really appreciate it.sseric
> takeshi10,
>
> >the same thread could always modify the list while
> iterating through it...
>
> Ok, that's what I didn't get and needed to know. I
> threw some Duke Dollars your way. Thanks for your
> help, I really appreciate it.
>
> sseric
no problem at all...
just another FYI, the iterator has a parameter, so you could do like this:
Iterator<String> it = list.iterator();
String a = iterator.next(); //no need to explicitly cast
that is, if youre familiar with generics...
Nice. I did not know that. Thanks again.sseric
