Is this BlockingQueue susceptible to deadlock?
I've been using this code as a queue that blocks on Dequeue()
until an element is enqueued. I've used this code for a few years now in several projects, all with no issues... until now. I'm seeing a deadlock in some code I'm writing now, and in i开发者_如何学JAVAnvestigating the problem, my 'eye of suspicion' has settled on this BlockingQueue<T>
. I can't prove it, so I figured I'd ask some people smarter than me to review it for potential issues. Can you guys see anything that might cause a deadlock in this code?
public class BlockingQueue<T>
{
private readonly Queue<T> _queue;
private readonly ManualResetEvent _event;
/// <summary>
/// Constructor
/// </summary>
public BlockingQueue()
{
_queue = new Queue<T>();
_event = new ManualResetEvent(false);
}
/// <summary>
/// Read-only property to get the size of the queue
/// </summary>
public int Size
{
get
{
int count;
lock (_queue)
{
count = _queue.Count;
}
return count;
}
}
/// <summary>
/// Enqueues element on the queue
/// </summary>
/// <param name="element">Element to enqueue</param>
public void Enqueue(T element)
{
lock (_queue)
{
_queue.Enqueue(element);
_event.Set();
}
}
/// <summary>
/// Dequeues an element from the queue
/// </summary>
/// <returns>Dequeued element</returns>
public T Dequeue()
{
T element;
while (true)
{
if (Size == 0)
{
_event.Reset();
_event.WaitOne();
}
lock (_queue)
{
if (_queue.Count == 0) continue;
element = _queue.Dequeue();
break;
}
}
return element;
}
/// <summary>
/// Clears the queue
/// </summary>
public void Clear()
{
lock (_queue)
{
_queue.Clear();
}
}
}
I think this could be your problem:
Thread 1 Thread 2
Dequeue
Enqueue
if (Size == 0) // Thread 1 gets the lock
lock (_queue) // Thread 2 has to wait
return _queue.Count // Thread 1 sees: Size == 0
_queue.Enqueue // Thread 2 gets the lock
_event.Set
_event.Reset // uh oh
_event.WaitOne // now Dequeue's going to block
// until Enqueue gets called again
// (even though queue isn't empty)
This code is broken in several ways. Here's one scenario. There's a race-condition between if (Size == 0)
and _event.Reset()
. An Enqueue might fire between the two, and its signal will be lost.
An unounded-length BlockingQueue is much more easily implemented with a semaphore.
I don't know about your requirements or what else your class does, but if you can use .NET 4, you might want to consider using ConcurrentQueue<T>
and BlockingCollection<T>
which, used together, should give you a blocking queue.
精彩评论