Mutual Exclusion with Mutexes

After I wrote this question: Producer/Consumer programs, I realized that I had forgotten to write a version using mutexes. I have now solved the Producer/Consumer problem using mutexes and implementing the tips I received from answers on that question. It appears to be working correctly; however, unlike my other programs, which would run for a while or until the buffer was full, this version tends to interlock the producer/consumer threads, so each character is consumed immediately after it is produced. Is this because the OS tends to preempt the thread when it releases the mutex if there is another waiting for it?

class Program
{
    const int BuffSize = 10;
    const int ValuesToProduce = 95;

    static void Main(string[] args)
    {
        var producerConsumer = new MutexProducerConsumer(BuffSize, ValuesToProduce);
        Thread p = new Thread(new ThreadStart(producerConsumer.Produce));
        Thread c = new Thread(new ThreadStart(producerConsumer.Consume));
        p.Start();
        c.Start();
    }
}

This is MutexProducerConsumer:

```
public class MutexProducerConsumer : ProducerConsumerBase
{
public MutexProducerConsumer(int BuffSize, int ValuesToProduce)
: base(BuffSize, ValuesToProduce) { }

volatile int Available = 0;
Mutex BufferLock = new Mutex(false);
Mutex IsFull = new Mutex(true);
Mutex IsEmpty = new Mutex(true);

public override void Produce()
{
for (int i = 0; i < _TotalNumberOfValues; i++)
{
while (Available == _BuffSize)
{
Console.WriteLine("Wait Producer:");
IsFull.WaitOne(1000);
}

BufferLock.WaitOne();
_Buffer[i % _BuffSize] = (char)(32 + i % 95);
Available++;
Console.WriteLine("Produced: {0}", _Buffer[i % _BuffSize]);
BufferLock.ReleaseMutex();

try
{
IsEmpty.ReleaseMutex();
}
catch

I'll answer your primary question first -

Why is it producing 1 char and then immediately consuming that char?

Look at the producing code carefully:

BufferLock.WaitOne();
_Buffer[i % _BuffSize] = (char)(32 + i % 95);
Available++;
Console.WriteLine("Produced: {0}", _Buffer[i % _BuffSize]);
BufferLock.ReleaseMutex();
IsEmpty.ReleaseMutex(); // Try catch removed, but throws quite often

What you're saying is:

• Get exclusive access to the buffer (slow)

• Add a character to the buffer

• Increment a shared field (in a potentially dangerous way)

• Release the buffer (slow)

• Potentially throw and catch an exception

At the same time your consumer is doing the analogous:

• Get exclusive access to the buffer (slow)

• Get a character from the buffer

• Decrement a shared field (in a potentially dangerous way)

• Release the buffer (slow)

• Potentially throw and catch an exception

If you start that work at the same time, you're going to end up in a situation where the consumer is first waiting for the producer to release the buffer, then the producer will be waiting for the consumer to release it on every iteration. The threads are basically playing tennis with the Mutex - back and forth, back and forth. Don't forget that acquiring and releasing a Mutex takes a pretty long time - a couple of microseconds - which might be contributing to the 1 produced, 1 consumed pattern as I think the code will be spending most of its time waiting for the mutex operations.

Notes on the implementation

Most importantly (and I think you know this) a Mutex is utterly overkill for inter thread synchronisation.

A Mutex can only be released by the thread that acquired it. So, the consuming thread will never be able to release the IsFull mutex because it will only ever be acquired by the producing thread. The same is true for the IsEmpty mutex - the producing thread will never be able to release it. I'm sure that you noticed the exception but rather than trying to figure out why it was happening, you've caught and ignored the exception. Having a reason to ignore all exceptions from an operation is quite rare - seeing an empty catch with no comment is usually a sign that something's not right.

Locks/mutexes etc. are expensive - minimize where possible and always hold the lock for the least amount of time possible.

Mutex implements IDisposable, as your MutexProducerProducerConsumer class holds references to them at the class level it should also implement IDisposable.

Your waits are just spin blocking. IsEmpty.WaitOne(1000) will wait up to one second to acquire the mutex.

while (Available < 1)
{
    Console.WriteLine("Wait Consumer:");
    IsEmpty.WaitOne(1000);
}

Your consumer will acquire the mutex and never release it. So your code is basically:

while (Available < 1)
{
    Console.WriteLine("Wait Consumer:");
}

Producer/consumer is more easily implemented with a queue of work. It's weird that your consumer has to know how many things the producer will create.

Some general style points:

This was mentioned in your previous question. I don't think you can actually fall foul of this here, but it's worth remembering that ++ and -- aren't atomic operations.

If the type of the RHS of an assignment is obvious, use var.

You don't need to create an explicit delegate here either:

Thread p = new Thread(new ThreadStart(producerConsumer.Produce));

can be more concisely written as:

var p = new Thread(producerConsumer.Produce);

StackExchange Code Review Q#69803, answer score: 11