CODEWITHSUNDEEP
c++concurrencysynchronizationmutexdeadlock
Eduard Rostomyan
Eduard Rostomyan

Reputation: 6546

Synchronized call of functions using std::condition_variable

I got addicted to learning concurrency again and tried to solve this problem.

In short, I have a class and 3 functions. I need to sync their calls (need to print FirstSecondThird).

It will become more clear with the code below:

std::function<void()> printFirst = []() { std::cout << "First"; };
std::function<void()> printSecond = []() { std::cout << "Second"; };
std::function<void()> printThird = []() { std::cout << "Third"; };

class Foo {
    std::condition_variable cv;
    bool mfirst,msecond;
    std::mutex mtx;
public:
    Foo() {
        mfirst = false;
        msecond = false;
    }

    void first(std::function<void()> printFirst) {
        std::unique_lock<std::mutex> l(mtx);
        printFirst();
        mfirst = true;
    }

    void second(std::function<void()> printSecond) {
        std::unique_lock<std::mutex> l(mtx);
        cv.wait(l, [this]{return mfirst == true; });
        printSecond();
        msecond = true;
    }

    void third(std::function<void()> printThird) {
        std::unique_lock<std::mutex> l(mtx);
        cv.wait(l, [this] {return (mfirst && msecond) == true; });
        printThird();
    }
};

int main()
{
    Foo t;

    std::thread t3((&Foo::third, t, printThird));
    std::thread t2((&Foo::second, t, printSecond));
    std::thread t1((&Foo::first, t, printFirst));

    t3.join();
    t2.join();
    t1.join();

    return 0;
}

And guess what my output is? It prints ThirdSecondFirst.

How is this possible? Isn't this code prone to DEADLOCK? I mean, when the first thread acquired the mutex in function second, wouldn't it wait forever because now as mutex is acquired we cannot change variable mfirst?

Upvotes: 1

Views: 89

Answers (1)

sparik
sparik

Reputation: 1201

You have a very subtle bug. 

std::thread t3((&Foo::third, t, printThird));

This line does not do what you expect. It initializes a thread with only one argument, printThird, instead of initializing with the 3 arguments you specified. That's because you accidentally used a comma expression. As a result, &Foo::third and t are just discarded, and the member functions are never even called.

The only functions that are called in your code are printFirst, printSecond, and printThird. Without any synchronization, they may print in arbitrary order. You may even get interleaved output.

std::thread t3{&Foo::third, t, printThird};

Here is the corrected version. The curly braces are for avoiding the compiler treating this as a function declaration because of most vexing parse

As a side note, as mentioned in the comments, you are using condition variables wrong.

Upvotes: 2

Related Questions