does Monitor.Wait Needs synchronization?

Question

Hi, I have developed a generic producer-consumer queue which pulses by Monitor in the following way:

the enqueue :

    public void EnqueueTask(T task)
    {
        _workerQueue.Enqueue(task);
        Monitor.Pulse(_locker);
    }

the dequeue:

private T Dequeue()
    {
        T dequeueItem;
        if (_workerQueue.Count > 0)
        {
               _workerQueue.TryDequeue(out dequeueItem);
            if(dequeueItem!=null)
                return dequeueItem;
        }
        while (_workerQueue.Count == 0)
            {
                Monitor.Wait(_locker);
        }
         _workerQueue.TryDequeue(out dequeueItem);
        return dequeueItem;
    }

the wait section produces the following SynchronizationLockException : "object synchronization method was called from an unsynchronized block of code" do i need to synch it? why ? Is it better to use ManualResetEvents or the Slim version of .NET 4.0?

Answer 1

Yes, the current thread needs to "own" the monitor in order to call either Wait or Pulse, as documented. (So you'll need to lock for Pulse as well.) I don't know the details for why it's required, but it's the same in Java. I've usually found I'd want to do that anyway though, to make the calling code clean.

Note that Wait releases the monitor itself, then waits for the Pulse, then reacquires the monitor before returning.

As for using ManualResetEvent or AutoResetEvent instead - you could, but personally I prefer using the Monitor methods unless I need some of the other features of wait handles (such as atomically waiting for any/all of multiple handles).

Answer 2

From the MSDN description of Monitor.Wait():

Releases the lock on an object and blocks the current thread until it reacquires the lock.

The 'releases the lock' part is the problem, the object isn't locked. You are treating the _locker object as though it is a WaitHandle. Doing your own locking design that's provably correct is a form of black magic that's best left to our medicine man, Jeffrey Richter and Joe Duffy. But I'll give this one a shot:

public class BlockingQueue<T> {
    private Queue<T> queue = new Queue<T>();

    public void Enqueue(T obj) {
        lock (queue) {
            queue.Enqueue(obj);
            Monitor.Pulse(queue);
        }
    }

    public T Dequeue() {
        T obj;
        lock (queue) {
            while (queue.Count == 0) {
                Monitor.Wait(queue);
            }
            obj = queue.Dequeue();
        }
        return obj;
    }
}

In most any practical producer/consumer scenario you will want to throttle the producer so it cannot fill the queue unbounded. Check Duffy's BoundedBuffer design for an example. If you can afford to move to .NET 4.0 then you definitely want to take advantage of its ConcurrentQueue class, it has lots more black magic with low-overhead locking and spin-waiting.