How to properly avoid ConcurrentModificationException when running two threads

Hi guys!

So, I’ve got the following setup:

A class “Game” and a class “RenderController”.
The Game class handles the entire game logic like moving, collision checking, and so on.
I do this to have my FPS completely seperated from the actual game speed.

RenderController renders down elements from Game’s lists like one called “Entities”.
As soon as I call entities.add(), I get the well-known “ConcurrentModificationException”.

I assume everyone of you had this problem once already or knows it at least.

So I got three ideas on how to solve this, but first I want to ask you guys which one would be the best, or if anyone got a better idea.

Idea #1:
Merge the Game and Render class. Would be the easiest, but worst idea imho.
Idea #2:
Before rendering, run .clone(); on each list and iterate the cloned list. I don’t know if this could perform bad in some later stage of the game.
Idea #3:
Wait for the Render class to finish rendering, and then add the elements to list. I can imagine that this would extremely slow down the game, as the Game class would be handling that.

I’d really appreciate some tips and experiences you’ve made regarding this exception and how to avoid it.

Thanks!

Literally just stop using threads.

To have FPS not impact physics speed in a 1 thread scenario, use a variable timestep (delta-time) for all your calculations.

if i understand it correctly then you some thread-unsafe collection and modify it while another thread is iterating over it.

there are alot ways to handle that. one is, dont use an iterator, which might be still a bit thread-unsafe and you might run into other exceptions like nullpointers or no-such-element, etc. :

instead of

[icode]for( Object element : things ) { […] }[/icode]

try

[icode]int size = things.size(); for(int i = 0, i < size; i++) { Object element = things.get(i); […] }[/icode]

personally i would just go ahead and replace the collection with one of the java.util.concurrent.

a list replacement might work with a http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/ConcurrentLinkedQueue.html
a hashmap may be a http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/ConcurrentHashMap.html.

pretty much a no-brainer.

o/

First of all, code organization has nothing to do with concurrency or parallelism (until things like coroutines). Splitting logic from rendering and etc. is a good idea, but separating threads of execution for the rendering loop from the logic loop is a separate issue.

I don’t know how you’ve tackled this as you seem to have some conflation of these and didn’t post any code.
If you do want concurrency then consider what is taking place: the logic computation produces the game state, and the renderer consumes it. For obvious reasons the logic cannot be modifying the same parts of memory the renderer is reading at the same time. So you could indeed have the logic feed a copy of the game state to the renderer.

If it turns out that isn’t fast enough for your machine or paranoia, find something that is equivalent but faster. One way to do start this analysis is to look for anything being wasted in some way. The most obvious waste with the copying technique is the consumed memory is simply thrown away by the renderer every update cycle. You could instead use a swapping technique: two segments of memory, one is being written into by the game logic while the other (the previous state) is being read by the renderer, when each is done they swap. Edit: the swapping still has to be thread-safe though.

That’s not good nor appropriate advice at all.

OT: Idea #2 is pretty standard. Keep in mind the RenderController should be absolutely pure as in a pure function, i.e., not having any side effects.

Idea #1 is probably the best and most suitable for the general case.

Somewhat related you may find this talk useful (particularly half-way through): https://www.youtube.com/watch?v=avwDj3KRuLc

Wow, thanks for the fast answers guys!

I tried basil_ attempt, and it worked like a charm.
Would not have thought that a basic for loop would not cause that error at all.

Also, thanks for the resources!

You just made the problem worse by supressing and ignoring it.

Instead, try to apply what BurntPizza and jonjava suggested: redesigning the code with the linked resources in mind.

No. Psylution made the application fault-tolerant.

Except it’s not. It now just crashes in 1 out of 100 runs, instead of almost always.
And thus, the concurrency issue is much harder to find.
All that the for-loop does is simply shrinking the critical section of code that MUST run atomically without modification. Just imagine the condition [icode]i < size[/icode] succeeding and then BOOM all elements in the List are concurrently removed and then the loop body executes [icode]things.get(i)[/icode].

Could you provide an explanation? I have always been told to use a variable time-step to maintain a consistent playing experience, regardless of FPS.

The very short answer is variable time steps are an all-around bad idea IMHO. The cost of a variable time-step update is much higher , likewise harder to write and it’s non-deterministic. The only downside of fixed time steps is that you either have to slow down or have specialized ‘tick’ routines for performing multiple update steps in one go.

isn’t the “proper” way to it a mix of both ?

update simulation by a fixed step each frame up to x times, fill up the frame-time and interpolate last step by the fraction which spills into next frame ?

Yes, it is simply key that the smallest unit of atomic update is constant in the simulation. How many units per render cycle and their variance matters much less, and only affects the render.

To generalize: All functionality that is visualization only, then variable rate is “The Correct Way®”. It’s also a pretty big knob one can turn to prevent and/or compensate for slowdowns.

Note that fixed-rate does not imply that all subsystems run at the same rate. As an example some fast-paced many entities games might run simulation at 10 Hz and the blending of the rendering hides that from the user. AIs might run faster or slower and there’s no requirement that active entities update every tick. The key thing is to be deterministic. Easy to understand, design, read, write and debug. Also there’s no requirement that all events occur at the fixed rate. If running simulation one could determine, say, where inside it a collision occurs and tweak the simulation state to reflect that. Still the result is deterministic (if you’re careful).

Note by deterministic I mean that in the weak way of locally…attempts to be floating-point deterministic across hardware, VMs and if the code is (for java) in interpreted vs. C1 vs. C2 vs. any future runtime compiler is completely nuts. And a complete misunderstanding of how to write floating-point code.