Reputation: 135
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
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
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