0

On the internet, I saw many example about Wait() and Pulse() and they used two while like in this example:

class MyQueue
{
    private Queue<string> queue = new Queue<string>();
    private const int CAPACITY = 3;
    public void Put(string element)
    {
        lock (this)
        {
            // first `while`
            while (queue.Count == CAPACITY)
            {
                Monitor.Wait(this);
            }
            queue.Enqueue(element);
            Console.WriteLine($"Put {element} ({queue.Count})");
            Monitor.Pulse(this);
        }
    }
    public string Take()
    {
        lock (this)
        {
            // second `while`
            while (queue.Count == 0)
            {
                Monitor.Wait(this);
            }
            string element = queue.Dequeue();
            Console.WriteLine($"Taked {element} ({queue.Count})");
            Monitor.Pulse(this);
            return element;
        }
    }
}

In the Main():

MyQueue queue = new MyQueue();
new Thread(new ThreadStart(() => {
    queue.Take();
    queue.Take();
})).Start();
new Thread(new ThreadStart(() => {
    queue.Put("a");
    queue.Put("b");
    queue.Put("c");
    queue.Put("d");
    queue.Put("e");
    queue.Put("f");
})).Start();

I think I understood the scenario of using Pulse() and Wait().

In the above example, I think it's ok to replace the two while with if. I tried and it also printed the same result.

Is it right? Thank you.

Nico Schertler
  • 32,049
  • 4
  • 39
  • 70
Phuc
  • 623
  • 1
  • 7
  • 11
  • Why are you using `lock` and `Monitor`? As far as I can tell, all of your thread safety is provided by `lock`. – ProgrammingLlama Jun 21 '19 at 02:33
  • 2
    Neither the implementation you showed, nor the suggested modifications are correct. The implementation will produce a deadlock whenever you try to add a fourth item (e.g., try to add a `Thread.Sleep(1000)` at the beginning of the read thread). – Nico Schertler Jun 21 '19 at 02:40
  • you'd probably better off using TPL and BufferBlock https://learn.microsoft.com/en-us/dotnet/standard/parallel-programming/how-to-write-messages-to-and-read-messages-from-a-dataflow-block – Keith Nicholas Jun 21 '19 at 02:46
  • After reading your replies guys... now I'm not sure what I want... I think I'm missing some basic about producer-consumer (someone contributed this tag) and I will take a look for it now. Anyway, thanks for your time guys. – Phuc Jun 21 '19 at 02:58
  • 3
    Today is a great day to learn the number one rule about multithreaded programming: "I tried it and it worked" tells you *absolutely nothing* about whether there are unhandled rare race conditions in your code. What you need to feel confident is "I understand all the rules of the memory model and I have a logical proof that this code is correct". Get out of the habit *right now* of believing that "trying it" is in any way sufficient, because it is not. – Eric Lippert Jun 21 '19 at 16:38
  • @EricLippert, i pressed the upvote, your reply really impressed me. – Phuc Jun 21 '19 at 16:56
  • 1
    @Messi: If you're implementing this in production (rather than for learning), I recommend using [System.Collections.Concurrent.BlockingCollection](https://learn.microsoft.com/en-us/dotnet/standard/collections/thread-safe/blockingcollection-overview). It's specifically designed to implement the Producer-Consumer pattern. By default it uses a `ConcurrentQueue`, but it can be used wrap any `IProducerConsumerCollection` (e.g., `ConcurrentStack`). – Brian Jun 24 '19 at 13:23

1 Answers1

2

In your exact example, probably it would be fine to do as you suggest. You have exactly one producer and one consumer, and so they should always operate in concert with each other to ensure a thread is woken only if its wait condition is resolved.

However:

  1. The producer and consumer implementations would not be safe to use if, if you have more than one of either the producer or consumer. This is because the threads could be racing, and one thread could be made runnable but then not scheduled until another thread has in some way invalidated the original resolution of the wait condition.
  2. While I'm skeptical that the .NET Monitor class is subject to the problem of spurious wake-ups — i.e. a thread in a wait state being woken due to some event other than an explicit wake by a cooperating thread (e.g. calling Monitor.Pulse()) — people who know concurrent programming and C# much better than I do have said otherwise (see e.g. Does C# Monitor.Wait() suffer from spurious wakeups?). And if you're at all concerned about spurious wake-ups, you'll want a loop instead of a simple if, to ensure that you recheck the wait condition before proceeding, just in case it wasn't actually satisfied before your thread was woken.

See also Eric Lippert's article Monitor madness, part two.

All that said, note that a producer/consumer scenario is much more easily implemented in modern .NET/C# by using BlockingCollection<T>. You can even include a maximum length for the queue when creating the collection to provide the "block if full" behavior seen in your code example.

Peter Duniho
  • 68,759
  • 7
  • 102
  • 136
  • 2
    Thanks for the shout out. Your final paragraph is in particular spot on: **use a higher-level construct written by experts to solve this problem**. – Eric Lippert Jun 21 '19 at 16:41
  • 1
    Also, in the article of mine you linked to I note that the comments from frequent SO contributor Stephen Cleary are particularly germane. He's far more of an expert in this area than I am. – Eric Lippert Jun 21 '19 at 16:42