Reputation: 5949
I'm trying to implement a thread-safe wrapper for a regular Queue class. But it seems that several threads may acquire the same lock. So my questions are:
1. How it could be?
2. How to avoid it?
3. Any useful advice are pleased
This is my queue:
public class SafeQueue<T> : SimpleQueue<T>
{
private readonly object sync = new object();
private readonly Queue<T> queue = new Queue<T>();
public override T Dequeue()
{
T item;
lock (sync)
{
Debug.Print("{1:mm ss ffff} {0} locked in dequeue", Thread.CurrentThread.ManagedThreadId, DateTime.Now);
item = queue.Dequeue();
}
Debug.Print("{1:mm ss ffff} {0} unlocked dequeue", Thread.CurrentThread.ManagedThreadId, DateTime.Now);
return item;
}
public override void Enqueue(T item)
{
lock (sync)
{
queue.Enqueue( item );
}
}
public override IEnumerator<T> GetEnumerator()
{
Queue<T> q;
lock (sync)
{
q = new Queue<T>(queue);
}
return ((IEnumerable<T>) q).GetEnumerator();
}
public override int Count
{
get
{
int c;
lock (sync)
{
Debug.Print("{1:mm ss ffff} {0} locked in count", Thread.CurrentThread.ManagedThreadId, DateTime.Now);
c = queue.Count;
}
Debug.Print("{1:mm ss ffff} {0} unlocked count. Ret {2}", Thread.CurrentThread.ManagedThreadId, DateTime.Now,c);
return c;
}
}
}
And this is a piece from where I use it (Work is a thread function):
private readonly SafeQueue<string> taskQueue = new SafeQueue<string>();
private volatile bool stop = false;
...
private void Work()
{
while ( !stop )
{
string dir = null;
if (taskQueue.Count > 0)
{
dir = taskQueue.Dequeue();
}
...
So, the problem basically is: two threads are acquire the lock on Count property and return a positive value. Since that they both try to dequeue an element.
Upvotes: 0
Views: 123
Reputation: 108860
One obvious bug is that an item might be dequeue between you checking the count, and the associated Dequeue
. For this reason threadsafe queues typically have an atomic TryDequeue
operations.
With such an operation, your code becomes:
while ( !stop )
{
string dir;
if(taskQueue.TryDequeue(out dir))
{
}
Your debug prints can be misleading too. You only print the "unlocked" line some time after the actual unlock occurs. So between the actual unlock and your debug output a different thread might enter the lock and print its "locked" line, leading to confusing output.
.net has two built in thread safe queues: ConcurrentQueue<T>
, which is non blocking, BlockingCollection<T>
which is blocking.
Upvotes: 4