Forum ‹ C# ‹ Thread safety and Locks

[GBurg]

Hi,

I am having a problem in my code with Queue's. It seems like my Queues are not thread safe, and sometimes Queue.DeQueue() returns a null object. How to handle this properly.

Right now I am checking all dequeued items whether they are Null or not, but it looks kinda stupid.

Can somebody help me with this?

[Henrik]

You need to use locks to make it threadsafe. The only important things to remember is that you have to use the same object for locking every time, and you want to minimize the amount of time spent inside a lock as much as possible.

Code: Select all

var queue = new Queue<Something>();
//...
queue.Enqueue(...);
//...
Something item = null;
lock (queue) {
   if (queue.Count > 0) {
      item = queue.Dequeue();
   }
}
if (item != null) {
   //...
}

[GBurg]

And do I also use locks on Lists and Dictionaries?

[Henrik]

And in case it wasn't clear, you have to do the count check and the dequeue inside the same lock, otherwise another thread could have modified the queue inbetween calls, and you don't want that.

And to minimize the code inside the lock, you put the logic of "could I get an object" outside, so you need that null-check where it is, even though it sounds pretty similar to what your code was doing. However, it's important to note that if you dequeue an empty queue, you get an exception, not null, and when you got a null, that's just bad, it probably means two threads are dequeing at the same time, and completely mangling it.

[GBurg]

Ok, seems I need some more knowledge on threads in c#. I am experiencing several thread errors, when stress testing my game. Is there some good article on c# and multithreading?

Also, a basic question: What happens when a queue is locked, and another thread wants to read/write from this queue?

[Henrik]

Multithreading isn't that hard. Different threads can generally read the same datastructures at the same time without problems, but you really only want one thread at a time to modify a datastructure, so if you have a queue or a list or a dictionary, and multiple threads access it, you need to lock all code that adds or removes objects from them. Enumerating a collection might also result in errors if it is modified while you're in the middle of that, so the safest bet there is to lock the structure, copy it, release lock, and then work with the copy.

When you use the lock statement, the first thread that reaches it will grab the lock, and proceed to run the code inside. Any other thread that reaches the lock statement will block until the first thread exits the code inside and releases the lock. Then the next thread will grab the lock, run the code, while the remaining threads block and wait for it, etc.

[GBurg]

Thanks, your helping a lot

Final question: When I have somethig like this structure:

Dictionary<int, List<GameEvent>> dictionary; // a nested list in a dictionary object

Would I need to lock the whole dictionary, or just dictionary[12], or both?

[GBurg]

A second final question:

Queue<Message> qmess;
lock (message_stack) {
qmess = new Queue<Message>(message_stack);
message_stack = new Queue<Message>();
}

Is how you want to deal with queues, when you want to empty a queue totally? qmess will then be used to process the queue, right?

[Henrik]

Final question: When I have somethig like this structure:

Dictionary<int, List<GameEvent>> dictionary; // a nested list in a dictionary object

Would I need to lock the whole dictionary, or just dictionary[12], or both?

That depends. You can lock on a single object when you modify either the dictionary as a whole or one of the lists in side it, or you can lock on the dictionary when you modify that, and each list when you modify those, but in that case you have to remember that while you're modifying a list, some other thread could modify the dictionary to remove that list from it.

It all comes down to how fine-tuned you want to control it. In general, you'll have one thread per player connected to a room, and there's not that many of those, so you most probably won't run into resource starvation or deadlocking, and you should be able to keep your code pretty simple and just have one big lock for a structure like that without any problems.

[Henrik]

A second final question:

Queue<Message> qmess;
lock (message_stack) {
qmess = new Queue<Message>(message_stack);
message_stack = new Queue<Message>();
}

Is how you want to deal with queues, when you want to empty a queue totally? qmess will then be used to process the queue, right?

Oh, you want your current thread to dequeue everything in message_stack, and then process those items without interference?

Reassigning the message_stack variable is bad, because that means the lock object changes, and you don't want that, you want all threads to lock on the same object. Instead you should call message_stack.Clear(); to remove all objects from that queue. But other than that I think it should work ok.

[GBurg]

Locking helped me quite alot, and got me rid from warning messages, when I put my server into stress situations. For people who want to read more on the subject of Locking:

http://www.albahari.com/threading/part2.aspx#_Locking

[garysimmons]

Thanks for the link. Some great info there to get me started