Why is std::condition_variable not waiting and immediately getting the lock again?

Question

I want to learn a bit threading this is my code.

Foo.hpp

#include <Windows.h>
#include <mutex>
#include <thread>
#include <iostream>


class Foo {
public:
    Foo(int i);

    int my_number = 0;

    static std::mutex mutex_open_table;

    std::condition_variable close_table_condition_variable; //Non Static on purpose
    bool notified_close_table = false;

    void OpenTable();
    void CloseTable();
};

Foo.Cpp

std::mutex Foo::mutex_open_table;


Foo::Foo(int i)
{
    my_number = i;
}


void Foo::OpenTable()
{
    while (true)
    {

        std::this_thread::sleep_for(std::chrono::seconds(1));
        std::unique_lock<std::mutex> lock(mutex_open_table);

        std::cout << "Open Table: " << my_number << std::endl;

        Sleep(1000);

        while (!notified_close_table) {  // loop to avoid spurious wakeups

            close_table_condition_variable.wait(lock);
        }
        std::cout << "Notified Table: " << my_number << std::endl;

        Sleep(1000);

        notified_close_table = false;
    }
}

void Foo::CloseTable()
{
    while (true)
    {
        std::this_thread::sleep_for(std::chrono::seconds(1));
        std::unique_lock<std::mutex> lock{ mutex_open_table };
        std::cout << "Close Table: " << my_number << std::endl;

        Sleep(1000);

        notified_close_table = true;
        close_table_condition_variable.notify_one();

    }
}

Main.Cpp

int main()
{

    Foo foo(1);
    Foo foo2(2);

    std::thread test2 = std::thread(&Foo::CloseTable, &foo);
    std::thread test3 = std::thread(&Foo::OpenTable, &foo);


    std::thread test4 = std::thread(&Foo::CloseTable, &foo2);
    std::thread test5 = std::thread(&Foo::OpenTable, &foo2);

    test2.join();
}

From my understanding/my brain thouhts

Thread with OpenTable can only print :

std::cout << "Open Table: " << my_number << std::endl;

After this message a CloseTable has to be executed so OpenTable can continue.

std::cout << "Close Table: " << my_number << std::endl;

And after this OpenTables std::condition_variable trys to lock the mutex again and has to print this message.

std::cout << "Notified Table: " << my_number << std::endl;

Thread with CloseTable Function can execute any time and multiple times.

While Starting the Programm there are times where After a "Open Table" there comes a "notified table"

Is there anything wrong with my thinking. Or anything wrong with my code?

TL:DR thought process is that for every "OpenTable" there needs to come a "Close Table"afterwards and afterwards a "Notified Table".

With the Remark that: In the meantime CloseTable can be executed and print "CloseTable" many times at any time.

Answer 1

Consider the following chain of events after program start:

1. notified_close_table is initialized as false.
2. CloseTable() is executed. This has no precondition. At the end, notified_close_table is set to true (the condition_variable::notify_one() is meaningless at this point).
3. OpenTable() is executed.
   1. First, "Open Table" is printed.
   2. Then the check in while (!notified_close_table) immediately fails (as notified_close_table was already set to true in the CloseTable() method previously).
   3. "Notified Table" is printed.
   4. notified_close_table is set to false.

So this will print "Close Table" => "Open Table" => "Notified Table".

If you want a "Close Table" to be printed between every Open/Notified, you have to move notified_close_table = false; in OpenTable() up, preceding the while loop:

    std::cout << "Open Table: " << my_number << std::endl;
    Sleep(1000);
    notified_close_table = false;
    while (!notified_close_table) {  // loop to avoid spurious wakeups
        close_table_condition_variable.wait(lock);
    }
    std::cout << "Notified Table: " << my_number << std::endl;

---

Additional improvements:

• After the fix, you can change the while loop to a do { ... } while loop to omit the first, unnecessary check

• In order to avoid unnecessary thread wakeups, condition_variable::notify_one() should be called after the mutex has been unlocked:

  void Foo::CloseTable()
  {
      while (true)
      {
          std::this_thread::sleep_for(std::chrono::seconds(1));
          {
              std::unique_lock<std::mutex> lock{ mutex_open_table };
              std::cout << "Close Table: " << my_number << std::endl;
              Sleep(1000);
              notified_close_table = true;
          }
          close_table_condition_variable.notify_one();
      }
  }