Maciej
Maciej

Reputation: 135

Why does printf cause deadlock with future.get while the cout does not?

I have some experience with MPI and CUDA and now I decided that it is high time to actually do some threading. I was learning the C++ Standard Library threading thingies and (based on a series of Youtube videos) I was building a simple piece of code which builds a job with std::packaged_task and sends it to job queue for worker threads to execute. Simple enough so far.

The problem started when I tried retrieving a result of the job via a future:

  printf_mutex.lock();
  printf("MAIN: Result of %i! is %i\n", 6, future_result_of_packaged_task.get()); // this causes deadlock!
  printf_mutex.unlock();

This locks the code forever! But this works:

  int mah_result = future_result_of_packaged_task.get();
  printf_mutex.lock();
  printf("MAIN: Result of %i! is %i\n", 6, mah_result ); // this is okay
  printf_mutex.unlock();

As does this (which is what the youtuber did):

std::cout << future_result_of_packaged_task.get() << "\n"; //this is okay

WHY DOES PRINTF() FAIL WHILE THE COUT WORKS CORRECTLY?

I think understanding this problem could be very educational.

The entire code is simple enough (some libraries are not needed since i just lazy copypasted them from a previous toy code, but who cares):

#include <cstdio>
#include <thread>
#include <string>
#include <mutex>
#include <condition_variable>
#include <iostream>
#include <future>
#include <deque>

int factorial(int N, std::mutex& printf_mutex)
{
  int result = 1;
  for (int i = N; i > 1; --i) result *= i;
  printf_mutex.lock();
  printf("FACTORIAL: Result of %i! is %i\n", N, result);
  printf_mutex.unlock();
  return result;
}

void worker_thread( std::deque< std::packaged_task<int()> >& task_queue, std::mutex& task_queue_mutex, std::condition_variable& task_queue_cv, std::mutex& printf_mutex )
{
  std::unique_lock<std::mutex> task_queue_mutex_lock(task_queue_mutex);
  task_queue_cv.wait(task_queue_mutex_lock, [&](){return !task_queue.empty();} );
  printf_mutex.lock();
  printf("WORKER: I'm not sleeping anymore!\n"); // this is okay
  printf_mutex.unlock();
  std::packaged_task<int()> my_task = std::move( task_queue.front() );
  task_queue.pop_front();
  my_task();
}

int main()
{
  std::mutex printf_mutex;
  std::mutex task_queue_mutex;
  std::deque< std::packaged_task<int()> > task_queue;
  std::condition_variable task_queue_cv;

  std::thread a_thread( worker_thread, std::ref(task_queue), std::ref(task_queue_mutex), std::ref(task_queue_cv), std::ref(printf_mutex) );
  std::this_thread::sleep_for(std::chrono::seconds(1));

  std::packaged_task<int()> a_task( bind(factorial, 6, std::ref(printf_mutex)) );
  std::future<int> future_result_of_packaged_task = a_task.get_future();

  task_queue_mutex.lock();
  task_queue.push_back(std::move(a_task));
  task_queue_mutex.unlock();
  task_queue_cv.notify_one();
  printf_mutex.lock();
  printf("MAIN: Notification sent!\n"); // this is okay
  printf_mutex.unlock();

  //std::cout << future_result_of_packaged_task.get() << "\n"; //this is okay

  int mah_result = future_result_of_packaged_task.get();
  printf_mutex.lock();
  printf("MAIN: Result of %i! is %i\n", 6, mah_result ); // this is okay
  printf_mutex.unlock();

  printf_mutex.lock();
  //printf("MAIN: Result of %i! is %i\n", 6, future_result_of_packaged_task.get()); // this causes a deadlock!
  printf_mutex.unlock();

  a_thread.join();
  return 0;
}

Yes, I hate C++ iostream and yes i hate the std::locks, their mere existence offends the Occam's Razor. I also use horrible naming scheme for my toy codes. None of that matters for the question tough.

EDIT: So, the solution to the puzzle is not obvious from the accepted answer. I want to make it clear: 1. Protecting the cout with printf_mutex makes it fail as well as printf. This suggests that the problem is either future.get() interfering with output mechanisms on my machine or the problem being a mutex clash / a race. When in doubt, always suspect a race, and notice that:
2. future.get() is a blocking function. I effectively locked a mutex and went to sleep, which is asking for races. Where can that race happen tough? By experiment we know that it never happens in the worker thread. Where else could it happen?
3. The answer is that the factorial also tries to lock printf_mutex and fails because the main always locks it first and then goes to sleep in future.get()

The accepted answer is the one which provided the strongest/most complete clue.

Upvotes: 0

Views: 986

Answers (2)

Jo&#227;o Paulo
Jo&#227;o Paulo

Reputation: 6700

You're having a race condition between worker_thread lock and the lowest printf_mutex.lock() (you can see it by placing a std::this_thread::sleep_for(std::chrono::seconds(1)); before its printf_mutex.lock();) and also, if, by luck, worker_thread wins, you're having a mutex logic problem between the locking inside factorial and the lowest printf_mutex.lock(). factorial will be locked forever, because it's called after printf_mutex been locked. Either you lock inside factorial or lock before the outside printf.

Working or not with std::cout or with printf is just luck. std::cout can be slower or faster than printf depending on your optimization directive. And note that you're using std::cout without printf_mutex.lock(); before, that's why it's working. Also, in the printf case that works, you're calling .get() before locking, that's why it works. Fix the race conditions and lock logics, then both printf and std::cout shall work.


Obs.: Prefer using RAII lock pattern and also consider using std::recursive_mutex for same thread locks, for printing, it can be very helpful.

Upvotes: 0

Alan Birtles
Alan Birtles

Reputation: 36488

you're holding printf_mutex so the task can't complete and future_result_of_packaged_task.get() never returns. Your other examples don't hold the mutex whilst calling get so don't deadlock.

Upvotes: 3

Related Questions