Multithreading application has no output

Question

I am doing an application which handle multithreading for exercise.Let's suppose that we have 10 cars, and there is a parking which may contain at maximum 5 cars.If a car can't park it waits until there is a free room.
I am doing this with c++11 threads:

#include <iostream>
#include <thread>
#include <mutex>
#include <condition_variable>

using namespace std;

int cars=0;
int max_cars=5;
mutex cars_mux;
condition_variable cars_cond;

bool pred()
{
    return cars< max_cars;
}

void task()
{
    unique_lock<mutex> lock(cars_mux);
    while(true)
    {
        cars_mux.lock();
        cars_cond.wait(lock,pred);
        cars++;
        cout << this_thread::get_id() << " has parked" << endl;
        cars_mux.unlock();
        this_thread::sleep_for(chrono::seconds(1));  // the cars is parked and waits some time before going away
        cars_mux.lock();
        cars--;
        cars_cond.notify_one();
        cars_mux.unlock();
    }
}

int main(int argc, char** argv)
{
    thread t[10];
    for(int i=0; i<10; i++)
        t[i]=thread(task);
    for(int i=0; i<10; i++)
        t[i].join();
    return 0;
}

The problem is that there is no output, it seems like all threads are blocked waiting.

score 3 · Accepted Answer · edited May 23 '17 at 12:12

3

There are 2 issues here:

Firstly, when you construct your lock object

unique_lock<mutex> lock(cars_mux);

then cars_mux is locked. So it is an error (undefined behaviour) to try and lock cars_mux again in the same thread, which is what you attempt to do immediately inside the while loop at

cars_mux.lock();

Secondly, there is no way for the threads to join since there is no way to exit the while(true) loop in task - cars will continue to park forever! You don't need the while loop at all.

If you remove the first cars_mux.lock();, the corresponding unlock attempt at the end of the while loop, and the while loop itself, you should get the desired behaviour.

edited May 23 '17 at 12:12

Community

1
1

answered Jul 05 '12 at 22:50

Fraser

74,704
20
238
215

3

In addition to Fraser's good advice, you should lock/unlock your mutex by using the `unique_lock` instead of the mutex directly: `lock.lock()` and `lock.unlock()`. This will ensure that if an exception is thrown in your loop, `lock` will do the right thing as the exception propagates out, because it will know whether or not `cars_mux` is locked. – Howard Hinnant Jul 05 '12 at 23:50

Bob Miller · Answer 2 · 2012-08-29T16:08:39.947

Fraser's answer is great, but when I looked at this I felt a working example would be nice.

I made some changes:

Personally, I like to use RAII to lock/unlock so that you don't forget to do so (even if it means extra scopes). In the past, I've also had race conditions that I couldn't figure out, and switching to the RAII approach has often made those disappear as well... It's just easier, so do it ;)
I like to see when the cars leave, so I add I/O for that too.

Here's the working code example for the stated problem. FYI, I use clang 3.1 with libc++:

#include <iostream>
#include <thread>
#include <mutex>
#include <condition_variable>
#include <chrono>

using namespace std;

int cars=0;
int max_cars=5;
mutex cars_mux;
condition_variable cars_cond;

bool pred()
{
    return cars < max_cars;
}

void task()
{
    {
        unique_lock<mutex> carlock(cars_mux);
        cars_cond.wait(carlock,pred);
        cars++;
            cout << "Thread " << this_thread::get_id() 
                 << " has parked. There are " << cars << " parked cars." << endl;
    }

    this_thread::sleep_for(chrono::seconds(1));

    {
        unique_lock<mutex> carlock(cars_mux);
        cars--;
        cout << "Thread " << this_thread::get_id() 
             << " has left. There are " << cars << " parked cars." << endl;
        cars_cond.notify_one();
    }
}

int main(int argc, char** argv)
{
    const int NumThreads = 10;
    thread t[NumThreads];
    for(int i=0; i<NumThreads; i++)
        t[i]=thread(task);
    for(int i=0; i<NumThreads; i++)
        t[i].join();
    return 0;
}

Edit: Simplified code as per Rami Al Zuhouri's suggestion.

The output always occurs when the mutex is locked, I don't see the reason for a mutex for I/O.I tried Fraser's suggestions and the code does the job. — Ramy Al Zuhouri, Jul 07 '12 at 13:55
@RamyAlZuhouri you're absolutely right. I had overcomplicated things, so I've edited the code to simplify it back down. — Bob Miller, Jul 07 '12 at 20:38

Multithreading application has no output

2 Answers2