Producer/Consumer

  • Thread starter Thread starter Guest
  • Start date Start date
G

Guest

It seems to me that the MSDN code for synchronizing a producer and consumer
thread at http://msdn2.microsoft.com/en-us/library/yy12yx1f.aspx is
completely wrong. There is no reason to assume that each call to
NewItemEvent.Set() will release a consumer thread (as explicitly mentioned in
the AutoResetEven documentation), which means that the consumer threads won't
necessarily know that there are items on the queue.

Is this correct?

Mike
 
Hi Mike,

As for the AutoResetEvent docs I assume that you're referring to this:

"There is no guarantee that every call to the Set method will release a
thread. If two calls are too close together, so that the second call occurs
before a thread has been released, only one thread is released. It is as if
the second call did not happen."

"AutoResetEvent Class"
http://msdn2.microsoft.com/en-us/library/system.threading.autoresetevent.aspx

I think it means that two serial calls to Set may only release one thread if
they are executed too close together (time-wise).

If you're referring to context switches, I don't think that calling Set on
an EventWaitHandle automatically causes a context switch, especially on a
single processor machine. Set signals blocking threads that they can run
whenever Windows decides to try and let them. That could be immediately, or
whenever Windows decides to interrupt the current process, AFAIK.

The example is written for one and only one consumer thread, which waits
until either one of the _newItemEvent or _exitThreadEvent EventWaitHandles
is set.

The code guarantees that there will be at least one item on the queue
whenever the consumer attempts to dequeue one. This is because the
_newItemEvent is set by the producer after enqueueing an item, and the
consumer only attempts to dequeue one item at a time, waiting on
_newItemEvent before trying again. When Windows allows the consumer to run,
whenever that may be, there will always be at least one item to be dequeued.
 
That is what I'm referring to. I agree that there will always be at least one
item to be dequeued when a thread is woken up, so the dequeue will never
fail. However, it is perfectly possible that all threads will be sleeping
even though there remain items in the queue (because the number of thread
wake-ups that occur may be less than the number of times the event is set).
The bottom-line is this is a completely busted implementation of a core
multithreading algorithm. Kind of embarrassing to see that up on MSDN. Makes
me wonder about the quality of the threading code in MS products :/

Mike
 
Hi Mike,

Yes, I see what you mean now, though the problem isn't with Set. The
problem is that the producer queues 20 items at a time as an atomic
operation, but the consumer is only prepared to dequeue one at a time.
Dequeuing 20 as such would fix that problem:

while (_queue.Count > 0)
_queue.Dequeue();

A potential problem is that the EventWaitHandle used to exit the application
can interrupt the consumer, leaving items to remain in the queue, but that
may be acceptable depending upon the requirements of the application.

Anyway, Jon Skeet's implementation seems much better [link posted by Brian].
Using Monitor.Pulse makes sense and the code is more legible :)
 
Back
Top