Threading problem with Monitor.Wait() and Monitor.Pulse()

Question

I have a producer-consumer scenario in ASP.NET. I designed a Producer class, a Consumer class and a class for holding the shared objects and responsible for communication between Producer and Consumer, lets call it Mediator. Because I fork the execution path at start-up (in parent object) and one thread would call Producer.Start() and another thread calls Consumer.Start(), I need to pass a reference of Mediator to both Producer and Consumer (via Constructor). Mediator is a smart class which will optimize many things like length of it's inner queue but for now consider it as a circular blocking queue. Producer would enqueues new objects to Mediator until the queue gets full and then Producer would block. Consumer dequeues objects from Mediator until there's nothing in the queue. For signaling between threads, I implemented two methods in Mediator class: Wait() and Pulse(). The code is something like this:

Class Mediator
{
  private object _locker = new object();

  public void Wait()
  {
    lock(_locker)
      Monitor.Wait(_locker);
  }

  public void Pulse()
  {
    lock(_locker)
      Monitor.Pulse(_locker);
  }
}

// This way threads are signaling:

Class Consumer
{
  object x;
  if (Mediator.TryDequeue(out x))
    // Do something
  else
    Mediator.Wait();
}

Inside Mediator I use this.Pulse() every time something is Enqueued or Dequeued so waiting threads would be signaled and continue their work.

But I encounter deadlocks and because I have never used this kind of design for signaling threads, I'm not sure if something is wrong with the design or I'm doing something wrong elsewhere ?

Thanks

Answer 1

If you can use .NET 4 your best bet would be to use BlockingCollection<T> (http://msdn.microsoft.com/en-us/library/dd267312.aspx) which handles queueing, dequeuing, and limits on queue length.

Answer 2

Is it possible that the deadlock is occurring because Pulse doesn't store any state? This means that if the Producer calls Pulse before/after Consumer calls Wait, then the Wait will block. This is the note in the documentation for Monitor.Pulse

Also, you should know that object x = new object(); is extraneous - an out call will initialize x, so the object created will fall out of scope with the TryDequeue call.

Answer 3

Difficult to tell with the code sample supplied.

• Is the lock held elsewhere? Within Mediator?
• Are the threads just parked on obtaining the lock and not on the actual Wait call?
• Have you paused the threads in a debugger to see what the current state is?
• Have you tried a simple test with just putting a simple single value on a queue and getting it to work? Or is Mediator pretty complex at this point?

Until a little more detail is available in the Mediator class and your producer class, it's some wild guessing. It seems like some thread may be holding the lock when you don't expect it to. Once you pulse, you do need to free the lock in whatever thread may have it by exiting the "lock" scope. So, if somewhere in Mediator you have the lock and then call Pulse, you need to exit the outer most scope where the lock is held and not just the one in Pulse.

Answer 4

There is not much code here to go on, but my best guess is that you have a live-lock problem. If Mediator.Pulse is called before Mediator.Wait then the signal gets lost even though there is something in the queue. Here is the standard pattern for implementing the blocking queue.

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

  public void Enqueue(T item)
  {
    lock (m_Queue)
    {
      m_Queue.Enqueue(item);
      Monitor.Pulse(m_Queue);
    }
  }

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

Notice how Monitor.Wait is only called when the queue is empty. Also notice how it is being called in a while loop. This is not strictly necessary in this case. An if check could have been used instead, but the while loop is the standard idiom when using Monitor.Wait because it is the only safe mechanism when using Monitor.PulseAll as opposed to Monitor.Pulse.

Answer 5

Nothing is wrong with design.

Problem raises when you use Monitor.Wait() and Monitor.Pulse() when you don't know which thread is going to do it's job first (producer or consumer). In that case using an AutoResetEvent resolves the problem. Think of consumer when it reaches the section where it should consume the data produced by producer. Maybe it reaches there before producer pulse it, then everything is OK but what if consumer reaches there after producer has signaled. Yes, then you encounter a deadlock because producer already called Monitor.Pulse() for that section and would not repeat it. Using AutoResetEvent you sure consumer waits there for signal from producer and if producer already has signaled before consumer even reaches the section, the gate is open and consumer would continue.

It's OK to use Monitor.Wait() and Monitor.Pulse() inside Mediator for signaling waiting threads.

Answer 6

Can you refactor to a normal consumer/ producer queue? That could then handle enqueing and dequing and thread-signalling in a single class, so no need to pass around public locks. Dequeing process could then be handled via a delegate. I can post an example if you wish.