Skip to main content
gamedev.net
🔒 Locked

[java] ArrayList and ConcurrentModificationException

Started by BattleMetalChris 14 replies 3.3k views
Original Post
BattleMetalChris
BattleMetalChris
I keep getting ConcurrentModificationException with ArrayLists I'm using. I'm not using threads though so I'm not quite sure why it's being thrown.

I do have a JFrame that accesses them during its Paint() method though - does the window run in it's own thread and that's why it's causing them?

I've not done much work with multithreading, bnut from what I've read, would adding syncronized(whateverArrayListImTryingToAcccess) blocks solve this?

Also what operations would need these blocks, everything or just .add and .remove?
Antheus
Antheus
Quote:
Original post by BattleMetalChris
does the window run in it's own thread
Yes. AWTEventQueue IIRC.

Quote:
I've not done much work with multithreading, bnut from what I've read, would adding syncronized(whateverArrayListImTryingToAcccess) blocks solve this?

Maybe.

Quote:
Also what operations would need these blocks, everything or just .add and .remove?


All operations that change internal state. Or use java.util.Vector instead, that one is fully synchronized.
BattleMetalChris
BattleMetalChris
I've just tried changing it to a java.util.Vector and I still get the error. It's being thrown by java.util.AbstractList
Antheus
Antheus
Quote:
Original post by BattleMetalChris
I've just tried changing it to a java.util.Vector and I still get the error. It's being thrown by java.util.AbstractList


for (Element e : myList) {  if (some_condition()) myList.add(x);}


This can trigger concurrent modification. for loop uses an iterator internally, so it requires that list doesn't change inside a for loop.
BattleMetalChris
BattleMetalChris
I have a few of them, but don't use add or remove, it's just a list of Entities for each element of which I call update(deltaTime).

I've fixed it for now using a try/catch with a blank catch block, which isn't the tidiest way of dealing with things I know, but it seems to work.
Antheus
Antheus
Quote:
Original post by BattleMetalChris

I've fixed it for now using a try/catch with a blank catch block
That did not fix anything, just exceptions aren't reported anymore, meaning that something now doesn't work correctly. Trying to debug that will become impossible.

Trying to actually solve such problem would involve at least posting source code of methods mentioned in exception stack trace.
Daniel L
Daniel L
It sounds like there's an iterator working in one thread while some mutating method is called from another thread. Try
synchronized (myVector) {    for (Element e : myVector) {        // Any code except add, remove, etc.    }}
Website · Blog
FlaiseSaffron
FlaiseSaffron
Quote:
Original post by Antheus
Quote:
Original post by BattleMetalChris

I've fixed it for now using a try/catch with a blank catch block
That did not fix anything, just exceptions aren't reported anymore, meaning that something now doesn't work correctly.

*laughs* It's the equivalent of putting your hands over your ears and yelling "lalalala, I don't hear you!" when someone says there's a problem. You did notice how your program still didn't do quite what you were expecting when you added that empty try/catch block, right?

Anyway, what Daniel suggested should work fine, though we may be able to give better suggestions if you post the code where you modify and access your vector or list.
rip-off
rip-off
Despite the name, ConcurrentModificationException has nothing to do with Threads. It just means that someone else has modified the structure of the List while you are iterating over it. It has nothing to do with synchronised, that cannot help if the modification is occurring in the same thread.

Antheus's example code demonstrates this. You might find that your setup is more complicated. You might iterate through a List, call a function on each element, and somewhere inside that function trigger another call which adds or removes elements from the list.

The first step is to find all code that iterates across and/or structurally modifies the list. The next step is to figure out a system for doing this in a mutually exclusive fashion (again I'm not talking about threads). A simple way is to have a master list, a list of items to add and a list of items to remove. Once every logical "frame", you perform all the add/remove operations and then clear() these auxiliary lists. Another approach would be for the objects to contain a flag indicating that they wish to be removed from the list. You might still need an auxiliary list to safely manage adding objects to the list.
blog | twitter
Glass_Knife
Glass_Knife
Quote:
Original post by rip-off
Despite the name, ConcurrentModificationException has nothing to do with Threads. It just means that someone else has modified the structure of the List while you are iterating over it. It has nothing to do with synchronised, that cannot help if the modification is occurring in the same thread.


This is exactly right. I have written code on many occasions like this...

List list;
List temp;
for each item in list
if should remove item
add item to temp list

for each item in temp list
remove item from real list

If someone knows a better way, I would love to hear about it. There is always some cool new Java method or class that I learn about and think, "Wow, I wish I knew about that years ago."
Antheus
Antheus
Quote:
Original post by Glass_Knife

If someone knows a better way, I would love to hear about it. There is always some cool new Java method or class that I learn about and think, "Wow, I wish I knew about that years ago."


Iterator has remove().

But it becomes a complexity problem. Unless list is a linked list, then removing m elements individually will be up to O(n^2).

One way, rather than removing elements - only copy surviving ones. Then swap the lists. This requires one pass with O(n) worst-case regardless of container. Only complication is swapping the containers.

If order doesn't matter, then swap removed elements with those from back, finally remove last n elements.

Of course, the STL's removeif is ideal, but I don't think it's part of standard libraries. But there are various third-party versions.
Daniel L
Daniel L
Quote:
Original post by Glass_Knife
I have written code on many occasions like this...

List list;
List temp;
for each item in list
if should remove item
add item to temp list

for each item in temp list
remove item from real list

If someone knows a better way, I would love to hear about it.


In general, I prefer iterating a clone of the original list. It work very nicely in Python -- we just replace "L" with "L[:]". For Java, ArrayList has a clone method, but I think toArray is slightly nicer:
Collection<E> c;for (E x : c.toArray(new E[0]))    if (shouldBeRemoved(x))        c.remove(x);
Website · Blog
zoto
zoto
If you use the old style for loop it is possible to change the same collection you are iterating over. If you iterate backwards there is no need to correct the counter variable when an element is removed.
for (int i = c.size() - 1; i >= 0; i--)  if (shouldBeRemoved(c.get(i)))    c.remove(i);
Consty
Consty
Just use CopyOnWriteArrayList from the Java5 concurrent collection set. You'll be good to go. You can use it anywhere in any thread and you're safe.
Arikwex
Arikwex
The repaint() call invoking paintComponent is a THREAD. So basically, if you have code updating your game elements (ArrayList) and you are running the paintComponent method at the same time, this error is bound to show up!

I usually write my gameloops like this:

while(true)
{
FPS = getFPS();
update(FPS);
repaint();
sleep(20);
}

The only problem occurs when the repaint method takes longer than 20ms. If the repaint call takes longer than that, then the next update will begin and alter the list concurrently, thus throwing an error. Hope this helps.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.