• Post Reply Bookmark Topic Watch Topic
  • New Topic

ConcurrentModificationException  RSS feed

 
Jose Alvarez de Lara
Ranch Hand
Posts: 94
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi,

I am having troubles in the following method,



The reasson is that throws a java.util.ConcurrentModificationException when I call it
from inside another method in a Servlet.

The argument List<DataFile> dfList is declared as
List<DataFile> dfList = Collections.synchronizedList(new ArrayList<DataFile>())

The execution rules ok until a random point that the exception is throwed.

Any suggestion should be appreciated.

Kind regards,
Jose
 
Martin Vajsar
Sheriff
Posts: 3752
62
Chrome Netbeans IDE Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I see three issues in your code, however I might be wrong, because I don't know the context and meaning of your method, most importantly where the dfList variable comes from.

1) You're iterating over synchronized list on line 15. Even though the list is synchronized, the iteration as a whole isn't. Put the whole for loop into a synchronized(list) block.

2) You're replacing a list that was already in the map with the dfList given as a parameter. I think this might lead to race conditions in your code. I'd say you should iterate over dfList and put all items into the list you got from the map. If the ownership structure of your lists is more complicated that I expect, I may be wrong in this regard. The race condition is there in any case, though.

3) The lines 2 to 13 of your code seem to just look for the presence of htmlMessage in the map. Simple hashMap.get(htmlMessage) should do the same and at the same time your intent would be clear, this way it is not clear at all. Besides, the performance is horrible and the operation is utterly thread-unsafe. (If the reason is you didn't implement hashcode() in your HTMLMessage class, go forth and implement it. It is the only way to go.)

Moreover, it's possible that if two threads try to insert identical HTMLMessage into the map, both will succeed (and whoever executes last the statement on line 21, list inserted by the other threads will be lost). You should use putIfAbsent() - putIfAbsent(htmlMessage, dfList) would probably do it.

I may have missed some of your intents, so don't just blindly rewrite the code according to my advice, but I'm pretty sure your code is not thread-safe in these three aspects. Maybe someone else will prove me wrong?
 
Jose Alvarez de Lara
Ranch Hand
Posts: 94
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Martin,

Yes, you are right. This is my code after debugging,



This works fine.

Kind regards,
Jose
 
Martin Vajsar
Sheriff
Posts: 3752
62
Chrome Netbeans IDE Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Jose,

unfortunately, the new version of your code doesn't address any of the issues I've written about and still is not thread-safe: you synchronize on the iterator you got by calling keys.iterator(). Every call to the iterator() method creates a new instance (it is sort of inevitable, isn't it?). The synchronized (it) statement therefore does not prevent different threads from entering the synchronized section, because any two different threads will synchronize on different instances of the iterator. That the code now seems to work fine might be caused by a slight changes in timing between the two versions.

If you intent to write thread-safe applications, you really need to get better understanding of multithreaded programming. You can start here, but keep in mind that it will take a lot of practice to get good understanding of this topic.

Meanwhile try to rework your code to address all of the issues I've mentioned. I can help you a bit if you get stuck.
 
Jose Alvarez de Lara
Ranch Hand
Posts: 94
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Martin,

I have saved the Concurrency tutorial in my favourite’s folder of the browser.

It is clear I have to study about this stuff.

Regarding the method I have changed my code that means
I do not going to need it any longer.

Kind regards,
Jose
 
Consider Paul's rocket mass heater.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!