C# Producer/Consumer pattern

Question

I have simple one producer two consumers code as follow but the output are only C2 consuming. any bug in my code?

class Program
{
    static void Main(string[] args)
    {
        Object lockObj = new object();
        Queue<string>  queue = new Queue<string>();
        Producer p = new Producer(queue, lockObj);
        Comsumer c1 = new Comsumer(queue, lockObj, "c1");
        Comsumer c2 = new Comsumer(queue, lockObj, "c2");

        Thread t1 = new Thread(c1.consume);
        Thread t2 = new Thread(c2.consume);
        t1.Start();
        t2.Start();

        Thread t = new Thread(p.produce);
        t.Start();

        Console.ReadLine();
    } 
}
public class Producer
{
    Queue<string> queue;
    Object lockObject;
    static int seq = 0;
    public Producer(Queue<string> queue, Object lockObject)
    {
        this.queue = queue;
        this.lockObject = lockObject; 
    }

    public void produce()
    {
        while( seq++ <15) //just testinng 15 items
        {
            lock (lockObject)
            {
                string item = "item" + seq;
                queue.Enqueue(item);
                Console.WriteLine("Producing {0}", item);
                if (queue.Count == 1)
                { // first
                    Monitor.PulseAll(lockObject);
                }
            }
        }
    }

}

public class Comsumer
{
    Queue<string> queue;
    Object lockObject;
    string name;
    public Comsumer(Queue<string> queue, Object lockObject, string name)
    {
        this.queue = queue;
        this.lockObject = lockObject; 
        this.name = name;
    }

    public void consume()
    {
        string item;
        while (true)
        {
            lock (lockObject)
            {
                if (queue.Count == 0)
                { 
                    Monitor.Wait(lockObject);
                    continue; 
                }
                item = queue.Dequeue();
                Console.WriteLine(" {0} Comsuming {1}", name, item);
            }
        }
    }
}

and output is

Producing item1 c2 Comsuming item1

Producing item2 c2 Comsuming item2

Producing item3 c2 Comsuming item3

Producing item4 c2 Comsuming item4

Producing item5 c2 Comsuming item5

Producing item6 c2 Comsuming item6

Producing item7 c2 Comsuming item7

Producing item8 c2 Comsuming item8

Producing item9 c2 Comsuming item9

Producing item10 c2 Comsuming item10

Producing item11 c2 Comsuming item11

Producing item12 c2 Comsuming item12

Producing item13 c2 Comsuming item13

Producing item14 c2 Comsuming item14

Producing item15 c2 Comsuming item15

Answer 1

For testing purposes, try adding a time delay inside the consumer code. It may be the case that "consumption" is so fast that one consumer thread empties the queue before the other consumer thread has a chance.

(edit)

As I suspected, adding a

Thread.Sleep(500);

inside the consumer thread (to simulate some lengthy processing going on) results in both threads being utilized.

Answer 2

Your producer only calls Monitor.PulseAll when the queue count is equal to 1 which will not be very often as nothing of substance is being done by the producer, this means that the first consume thread through the gate gets to dequeue the first item, the second consume thread will see no items in the queue and so hit the Monitor.Wait, and the Pulse won't happen again (probably until all but the last item is left) so that second thread will sit in that wait infinitely.

Answer 3

Added Thread.Sleep(500); in Consumer.comsume

then I have the following,

c2 Comsuming item1 c1 Comsuming item2 c2 Comsuming item3 c1 Comsuming item4 c2 Comsuming item5 c1 Comsuming item6 c2 Comsuming item7 c1 Comsuming item8 ..... the resule is not uncertain after adding Sleep.

Answer 4

First, I can't reproduce your problem, here both threads consume some of the items. I guess your machine is faster but adding Sleep like gw suggest will solve that. What I would also suggest is that you don't try to sync the producer, I mean let it queue items as fast as it can and let the consumers sync to see who handles each item. I made a quick modification and it seems to be working fine:

static void Main()
    {
        Object lockObj = new object();
        Queue<string> queue = new Queue<string>();
        Producer p = new Producer(queue);
        Comsumer c1 = new Comsumer(queue, lockObj, "c1");
        Comsumer c2 = new Comsumer(queue, lockObj, "c2");

        Thread t1 = new Thread(c1.consume);
        Thread t2 = new Thread(c2.consume);
        t1.Start();
        t2.Start();

        Thread t = new Thread(p.produce);
        t.Start();

        Console.ReadLine();
    }
}
public class Producer
{
    Queue<string> queue;
    static int seq;
    public Producer(Queue<string> queue)
    {
        this.queue = queue;
    }

    public void produce()
    {
        while (seq++ < 1000) //just testinng 15 items
        {
            string item = "item" + seq;
            queue.Enqueue(item);
            Console.WriteLine("Producing {0}", item);                
        }
    }
}

public class Comsumer
{
    Queue<string> queue;
    Object lockObject;
    string name;
    public Comsumer(Queue<string> queue, Object lockObject, string name)
    {
        this.queue = queue;
        this.lockObject = lockObject;
        this.name = name;
    }

    public void consume()
    {
        string item;
        while (true)
        {
            lock (lockObject)
            {
                if (queue.Count == 0)
                {
                    continue;
                }
                item = queue.Dequeue();
                Console.WriteLine(" {0} Comsuming {1}", name, item);
            }                
        }
    }
}

You may also add the sleep to slow down the consumer loops.

Answer 5

I ran you code and had spurts of c1 doing the consuming and spurts of c2. You may just want to check this link from msdn: How to: Synchronize a Producer and a Consumer Thread (C# Programming Guide)

Answer 6

I think your purpose is to have more than one consume threads working "in parallel". But your code is low efficient. The two consume threads are working sequentially essentially. The actual working code should be put outside the lock so the two consumer threads can run in real parallel. This improves runtime if you have multiple cores or even on a single core machine depending on the property of the work. Otherwise, there is actually no point to have more than one consume thread because anyway, all consume threads run in sequential.