concurrent modification error

[John Lincoln]

Hi,
I am using ListInterator interface for iterating through the vector datastructure i have. I have read from Sun docs :
"The Iterators returned by Vector's iterator and listIterator methods are fail-fast: if the Vector is structurally modified at any time after the Iterator is created, in any way except through the Iterator's own remove or add methods, the Iterator will throw a ConcurrentModificationException. Thus, in the face of concurrent modification, the Iterator fails quickly and cleanly, rather than risking arbitrary, non-deterministic behavior at an undetermined time in the future. The Enumerations returned by Vector's elements method are not fail-fast. "
My code is sunchronized while accessing vector.
But even then i get concurrent modification error. I do not see any bad behaviour ( i mean nothing wrong is happening)and programs is working as it should be, except this error?
Is there any way i can overcome this error ?
Please help.

Thanks in advance.
Praveen.

[Lakshmi Dasari]

Hi Kumar,
Could you post the source code for better understanding.

[John Lincoln]

ListIterator msgiterator = null;
 SpreadMessage spmsg = null;
   do{   
 msgiterator = SpaceSpreader.messageTable.listIterator();
                     
     if(messageTable == null){
      System.out.println("Message Table is null");
      }
      else{
         synchronized(SpaceSpreader.messageTable){
              while(msgiterator.hasNext()){
                try{
           spmsg = (SpreadMessage)msgiterator.next(); 
                   {System.out.println("Index am about to process :"
                   +SpaceSpreader.messageTable.indexOf(spmsg) );
                                    msgiterator.remove();
                                                                  } 
                {processMessage(spmsg);}
                 System.out.println("current size of messageTable  :"
                  + (SpaceSpreader.messageTable.size()));
                    }
              }
}
while(true);

[ March 26, 2003: Message edited by: kumar cheruvu ]
(edited by Cindy to format code)
[ March 31, 2003: Message edited by: Cindy Glass ]

[Peter den Haan]

There is one problem in immediate evidence: the iterator is retrieved outside the synchronized block, so there is an opportunity for another thread to modify messageTable between calling iterator() and entering the synchronized block.
Another probable problem is that the synchronization will only achieve anything if all the code that might modify messageTable is synchronized as well. And, of course, if processMessage() modifies messageTable you'll also get a ConcurrentModificationException.
The explicit synchronized block is a bit of a "code smell". What I would strongly suggest is not synchronizing on the Map itself, but rather wrapping that map inside your own class that exposes methods that do whatever you want to do. Synchronize the methods on your class. All of them, unless you know what you're doing.
Other things --the if (messageTable == null) will never execute, you'd get a NullPointerException from the previous statement. I would also strongly encourage you not to declare msgiterator or spmsg at the top; their scope becomes much too large which reduces the readability of your code. There is no reason why you wouldn't say

do { 
   if(messageTable == null){
      System.out.println("Message Table is null");
      break;
   }
   else {
      synchronized (SpaceSpreader.messageTable) {
      ListIterator msgiterator = SpaceSpreader.messageTable.listIterator();
      while (msgiterator.hasNext()) {
      try{
         SpreadMessage spmsg = (SpreadMessage)msgiterator.next();
         System.out.println("Index am about to process :"+SpaceSpreader.messageTable.indexOf(spmsg) );
         msgiterator.remove();
         processMessage(spmsg);
         System.out.println("current size of messageTable :"+ (SpaceSpreader.messageTable.size()));
      }
   }
} while(true);

There are no performance penalties associated with this. There's more strange stuff -- the endless do...while loop -- but I guess you know best what your code is supposed to do
- Peter

[John Lincoln]

Thanks Peter.