[.net] Thread Locking

Just an update.


I went with

for (int i = Reg.Count - 1; 0 < i; i--)
{
Update(reg);
}

How do people feel about this. Is there a chance at getting index out of bounds?


Thanks,

Your instincts were on the right track. Buffer the changes to the set, and apply them at the end of each iteration. Your loop will end up very similar to how it currently is.

while(true){     foreach(Region r in WorldRegions)          foreach(Entity e in r)               Update(e);     lock(something)     {          ApplyChanges();     }}....private void Add(Entity e){     lock(same something)     {          WorldRegion r = getRegion(e.Location);          QueueUpAdd(r,e);     }}

Assuming it is a threading issue at all. Changing the containers at all that are currently being iterated over with a foreach [whether in the thread they are being worked on, or in another] will result in an exception. The locking isn't necessary at all if 'add' is called in the same thread. QueueUpAdd just stores r and e, and calls r.Add(e) when ApplyChanges is called.

The problem isn't threading, its that you are trying to modify a collection while iterating through it. I strongly suggest you stick with your original plan of using a foreach and letting the error annoy you to death. Why? You do not want a collection to be modified while you are iterating through it. If it is modified, you may end up missing elements or going out-of-bounds.

One thing you can do is redesign the code to not do this. What you are doing is:

-Update world segment
---Update entities in world segment
-----Add new entities to world segment on entity update
---
-

(awesome chart, I know)

One way to fix this is... why are entities being added through an entity update? I know there are valid reasons to do this, but do you need to? One way you could get by it is by using a pool instead. When you want to add a new entity, you simply grab an unused one from the pool and "turn it on". Another is, like mentioned, continue what you are doing but just make temporary list to enumerate through every time.

It is not an easy problem, that is for sure. But I strongly suggest spending the time on it since this is not the first time you will run into an issue like this. Bypassing the error by removing the foreach is not a solution, even though it may look like it since it doesn't make the error.

I ran into something like this a while ago. I am making an ORPG so, when my NPCs die, they are simply set to "IsAlive=false". But when I want to dispose of them, completely removing them from the map forever (such as with a summoned NPC), it could run into this same issue. My solution was to give each map (was actually a property that went to a single queue in the World) a Queue that could be accessed by the entities. When I wanted a NPC, or anything for that matter to be disposed, I would place it in this queue. At the very end of my World's update, the queue was processed and every item in it was disposed. The result isn't super clean, but it works until I can clean up the code later. The important part is that it functions properly.

I would agree with what Drigovas said.

Have a function that iterates through your stuff doing updates.

At the end of that function (assuming it is never called in multiple threads), run through an Action queue (adds / deletes / moves / whatever) and apply the action queue changes to the list and then clear the queue.

This works best with two queues. One queue is the "active" queue and the other is the "working" queue. The function reads through the "working" queue so that actions can be executed to the "active" queue (through the Add function or whatever). Either before working on the action queue or after (your preference), it switches the "active" queue and the "working" queue... similar to DX Back Buffering.

HTH.

-E