Multithreading with Monitor enter/exit, wait/pulse, lock

Question

I am working at the Cigarette Smoker Problem.

I am only supposed to use the Monitor class. No Signals/Semaphores. (Yes this is for School, but not homework, just a free-to-do exercise for my practical test, and I really need to get prepared, so having this program work would help me a lot).

My problem is that I dont know which objects to "lock" etc. (as you can see I already tried alot with this and just random objects)

I have 4 Threads, 1 Dealer, 3 Smoker. I have a Dealer Class and a Smoker Class. At the moment, all my Smokers go to Monitor.Wait() and have like 1 or 2 ingridients and then they never get out of it again, even though I call Monitor.PulseAll() every time the Dealer places ingridients onto the table. I think this is because I am using the wrong objects as Parameters and I have absolutely no idea.

textbox and strings are mostly used to give the data out in my WPF class.

Smoker Class: please excuse the german variable names. (tabak = tabacco, papier = paper, streichhölzer = firethingies, zutat = ingridients, rauchzeit = smoketime)

class Raucher
{
    enum Zutaten { Tabak, Papier, Streichhölzer, Leer };

    public static Random r = new Random();
    int id;
    String status = "";
    public static int rauchzeit, rauchzeitvar, drehzeit, drehzeitvar;
    TextBox txtbox;
    Zutaten zutat1;
    Zutaten zutat2;
    Zutaten zutat3;
    public static Dealer dealer;


    public Raucher(Int32 id, Int32 rauchzeit, Int32 rauchzeitvar, Int32 drehzeit, Int32 drehzeitvar, TextBox status1, TextBox status2, TextBox status3, Dealer dealer)
    {
        this.id = id;
        Raucher.rauchzeit = rauchzeit;
        Raucher.rauchzeitvar = rauchzeitvar;
        Raucher.drehzeit = drehzeit;
        Raucher.drehzeitvar = drehzeitvar;
        Raucher.dealer = dealer;
        status = "Warten";
        switch (id)
        {
            case (0):
                txtbox = status1;
                zutat1 = Zutaten.Tabak;
                break;
            case (1):
                txtbox = status2;
                zutat1 = Zutaten.Papier;
                break;
            case (2):
                txtbox = status3;
                zutat1 = Zutaten.Streichhölzer;
                break;
        }
        zutat2 = Zutaten.Leer;
        zutat3 = Zutaten.Leer;
    }

    public void updateText()
    {
        try
        {
            txtbox.Dispatcher.BeginInvoke(
              System.Windows.Threading.DispatcherPriority.Normal
              , new System.Windows.Threading.DispatcherOperationCallback(delegate
              {
                  txtbox.Text = status;
                  switch (status)
                  {
                      case "Drehen":
                          txtbox.Background = Brushes.White;
                          break;
                      case "Rauchen":
                          txtbox.Background = Brushes.Green;
                          break;
                      case "Warten":
                          txtbox.Background = Brushes.Red;
                          break;
                  }

                  txtbox.UpdateLayout();
                  return null;
              }), null);
        }
        catch (Exception ex)
        {
            System.Diagnostics.Debug.WriteLine(ex.ToString());
        }
    }

    public static readonly object _locker = new object();

    public void Go()
    {
        while (true)
        {
            lock (_locker)
            {
                Console.WriteLine("Tabak: " + dealer.tabak);
                Console.WriteLine("Papier: " + dealer.papier);
                Console.WriteLine("Streichhölzer: " + dealer.streichhölzer);
                if (!dealer.tabak && !dealer.papier && !dealer.streichhölzer)
                {
                        Monitor.PulseAll(_locker);                
                }
                if (zutat1 == Zutaten.Tabak)
                {
                    if (dealer.papier && zutat2 == Zutaten.Leer)
                    {
                        dealer.takePapier();
                        zutat2 = Zutaten.Papier;
                    }
                    if (dealer.streichhölzer && zutat3 == Zutaten.Leer)
                    {
                        dealer.takeStreichhölzer();
                        zutat3 = Zutaten.Streichhölzer;
                    }
                    if (zutat2 == Zutaten.Papier && zutat3 == Zutaten.Streichhölzer)
                    {
                        status = "Drehen";
                        updateText();
                        Thread.Sleep(r.Next(drehzeit - drehzeitvar, drehzeit + drehzeitvar));
                        status = "Rauchen";
                        updateText();
                        Thread.Sleep(r.Next(rauchzeit - rauchzeitvar, rauchzeit + rauchzeitvar));
                        zutat2 = Zutaten.Leer;
                        zutat3 = Zutaten.Leer;
                    }
                    else
                    {
                        Monitor.Wait(_locker);
                    }
                }
                if (zutat1 == Zutaten.Papier)
                {
                    if (dealer.tabak && zutat2 == Zutaten.Leer)
                    {
                        dealer.takeTabak();
                        zutat2 = Zutaten.Tabak;
                    }
                    if (dealer.streichhölzer && zutat3 == Zutaten.Leer)
                    {
                        dealer.takeStreichhölzer();
                        zutat3 = Zutaten.Streichhölzer;
                    }
                    if (zutat2 == Zutaten.Tabak && zutat3 == Zutaten.Streichhölzer)
                    {
                        status = "Drehen";
                        updateText();
                        Thread.Sleep(r.Next(drehzeit - drehzeitvar, drehzeit + drehzeitvar));
                        status = "Rauchen";
                        updateText();
                        Thread.Sleep(r.Next(rauchzeit - rauchzeitvar, rauchzeit + rauchzeitvar));
                        zutat2 = Zutaten.Leer;
                        zutat3 = Zutaten.Leer;
                    }
                    else
                    {
                        Monitor.Wait(_locker);
                    }
                }
                if (zutat1 == Zutaten.Streichhölzer)
                {
                    if (dealer.papier && zutat2 == Zutaten.Leer)
                    {
                        dealer.takePapier();
                        zutat2 = Zutaten.Papier;
                    }
                    if (dealer.tabak && zutat3 == Zutaten.Leer)
                    {
                        dealer.takeTabak();
                        zutat3 = Zutaten.Tabak;
                    }
                    if (zutat2 == Zutaten.Papier && zutat3 == Zutaten.Tabak)
                    {
                        status = "Drehen";
                        updateText();
                        Thread.Sleep(r.Next(drehzeit - drehzeitvar, drehzeit + drehzeitvar));
                        status = "Rauchen";
                        updateText();
                        Thread.Sleep(r.Next(rauchzeit - rauchzeitvar, rauchzeit + rauchzeitvar));
                        zutat2 = Zutaten.Leer;
                        zutat3 = Zutaten.Leer;
                    }
                    else
                    {
                        Monitor.Wait(_locker);
                    }
                }
            }
        }
    }
}

Dealer Class:

class Dealer
{
    public static Random r = new Random();
    public Boolean tabak = false;
    public Boolean papier = false;
    public Boolean streichhölzer = false;
    public String zutaten;

    public Boolean isEmpty()
    {
        return !(tabak || papier || streichhölzer);
    }

    public void setTabak()
    {
        tabak = true;
    }

    public void setPapier()
    {
        papier = true;
    }

    public void setStreichhölzer()
    {
        streichhölzer = true;
    }

    public void takeTabak()
    {
        tabak = false;
    }

    public void takePapier()
    {
        papier = false;
    }

    public void takeStreichhölzer()
    {
        streichhölzer = false;
    }

    TextBox status;

    public Dealer(TextBox status)
    {
        this.status = status;
    }

    public static readonly object _locker = new object();

    public void Go()
    {
        while (true)
        {
            if (isEmpty())
            {
                lock (this)
                {
                    if (!tabak && !papier && !streichhölzer)
                    {
                        int zahl1 = r.Next(0, 3);
                        int zahl2 = r.Next(0, 3);
                        while (zahl1 == zahl2)
                        {
                            zahl2 = r.Next(0, 3);
                        }
                        switch (zahl1)
                        {
                            case (0):
                                setTabak();
                                break;
                            case (1):
                                setPapier();
                                break;
                            case (2):
                                setStreichhölzer();
                                break;
                        }
                        switch (zahl2)
                        {
                            case (0):
                                setTabak();
                                break;
                            case (1):
                                setPapier();
                                break;
                            case (2):
                                setStreichhölzer();
                                break;
                        }
                        updateText();
                        Monitor.PulseAll(this);
                    }
                }
            }
        }
    }

    public void updateText()
    {
        try
        {
            status.Dispatcher.BeginInvoke(
              System.Windows.Threading.DispatcherPriority.Normal
              , new System.Windows.Threading.DispatcherOperationCallback(delegate
              {
                  zutaten = "";
                  if (tabak)
                  {
                      zutaten += " Tabak ";
                  }
                  if (papier)
                  {
                      zutaten += " Papier ";
                  }
                  if (streichhölzer)
                  {
                      zutaten += " Streichhölzer ";
                  }
                  status.Text = zutaten;
                  status.UpdateLayout();
                  return null;
              }), null);
        }
        catch (Exception ex)
        {
            System.Diagnostics.Debug.WriteLine(ex.ToString());
        }
    }
}

Answer 1

There are several things wrong:

1. Both Smoker and Dealer class have their own _locker objects. Synchronization between Smoker and Dealer threads will not happen if they do not access the same synchronization primitive somehow.

2. In Dealer, you lock on "this", and pulse on it as well. It is meaningless, since no other thread will know about it, because only a single thread (dealer) uses it.

3. Locking on "this" or public properties/fields is bad practice. In larger software projects, such a practice could easily lead to dead-lock situation.

4. Do not make each class use synchronization objects of each other. Just let the Dealer have one synchronization object, which is used by the Smokers to synchronize access to Dealer resources. But it is actually also a very bad practice to have multiple classes messing around with the synchronization object.

5. Much better is to have one private synchronization object within the Dealer class, and you implement your application logic in such a way, so that synchronization can be handled internally in the Dealer class only, completely transparent to any Smoker.