The conditional variable is not working but after adding std::cout, it is working

Question

My project is consists of two threads: one main thread and the other thread which handles another window content. So, the when the main thread wants to ask the another windows to update itself it calls the draw function which is as follows:

void SubApplicationManager::draw() {

  // Zero number of applications which has finished the draw counter
  {
    boost::lock_guard<boost::mutex> lock(SubApplication::draw_mutex);
    SubApplication::num_draws = 0;
  }

  // Draw the sub applications.
  for (size_t i = 0; i < m_subApplications.size(); i++)
    m_subApplications[i].signal_draw();

  // Wait until all the sub applications finish drawing.
  while (true){
    boost::lock_guard<boost::mutex> lock(SubApplication::draw_mutex);
    std::cout << SubApplication::num_draws << std::endl;
    if (SubApplication::num_draws >= m_subApplications.size()) break;
  }

}

The draw function just signals the other thread that a new task is received.

void SubApplication::signal_draw() {

  task = TASK::TASK_DRAW;
  {
    boost::lock_guard<boost::mutex> lock(task_received_mutex);
    task_received = true;
  }
  task_start_condition.notify_all();

}

The body of other thread is as follows. It waits for the task to arrive and then start to process:

void SubApplication::thread() {

  clock_t start_time, last_update;
  start_time = last_update = clock();

  //! Creates the Sub Application
  init();

  while (!done)                                                       // Loop That Runs While done=FALSE
  {
      // Draw The Scene.  Watch For ESC Key And Quit Messages From DrawGLScene()
      if (active)                                       // Program Active?
      {
        // Wait here, until a update/draw command is received.
        boost::unique_lock<boost::mutex> start_lock(task_start_mutex);
        while (!task_received){
          task_start_condition.wait(start_lock);
        }

        // Task received is set to false, for next loop.
        {
          boost::lock_guard<boost::mutex> lock(task_received_mutex);
          task_received = false;
        }

        clock_t frame_start_time = clock();

        switch (task){
        case TASK_UPDATE:
          update();
          break;

        case TASK_DRAW:
          draw();
          swapBuffers();
          break;

        case TASK_CREATE:
          create();
          break;

        default:
          break;
        }

        clock_t frame_end_time = clock();
        double task_time = static_cast<float>(frame_end_time - frame_start_time) / CLOCKS_PER_SEC;

      }
  }
}

The problem is that if I run the code as it is, it never runs the other thread with task = TASK::TASK_DRAW; but if I add a std::cout << "Draw\n"; to the beginning of SubApplication::draw(), it will work as it should. I am looking for the reason which it is happening and what is the usual way to fix it?

Answer 1

boost::lock_guard<boost::mutex> lock(task_received_mutex);
task_received = true;

Okay, the task_received_mutex protects task_received.

    boost::unique_lock<boost::mutex> start_lock(task_start_mutex);
    while (!task_received){
      task_start_condition.wait(start_lock);
    }

Oops, we're reading task_received without holding the mutex that protects it. What prevents a race where one thread reads task_received while another thread is modifying it? This could immediately lead to deadlock.

Also, you have code that claims to "Wait until all the sub applications finish drawing" but there's no call to any wait function. So it actually spins rather than waiting, which is awful.

Answer 2

As a starter, signal the task_start_condition under the task_start_mutex lock.

Consider locking that mutex during thread creation to avoid obvious races.

Third: it seems you have several mutexes named for "logical tasks" (draw, start). In reality, however, mutexes guard resources, not "logical tasks". So it's good practice to name them after the shared resource they should guard. _(In this case I get the impression that a single mutex could be enough/better. But we can't tell for sure from the code shown)).