Synchronized call of functions using std::condition_variable

Question

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?

[`cv.wait()` unlocks the mutex](https://en.cppreference.com/w/cpp/thread/condition_variable/wait) while waiting, allowing another thread to acquire it — Remy Lebeau, Apr 07 '20 at 18:20
thanks @RemyLebeau, now I understand that it couldnt cause deadlock. But how it can call Third before all above? Shouldnt cv wait for booleans to become true? — Eduard Rostomyan, Apr 07 '20 at 18:22
Your code is not calling `cv.notify_all()` or `cv.notify_one()` to notify waiting threads that `cv` has been signaled so they can stop waiting and re-acquire the mutex lock to check their predicate conditions. After calling `mfirst = true;` and `msecond = true;` you need to notify `cv`. Calling `cv.wait()` without `cv.notify_...()` defeats the whole purpose of using a `conditional_variable` at all — Remy Lebeau, Apr 07 '20 at 18:24
still, I cannot understand how the thread is passing this line: cv.wait(l, [this] {return (mfirst && msecond) == true; }); — Eduard Rostomyan, Apr 07 '20 at 18:26

sparik · Accepted Answer · 2020-04-07T21:03:46.030

2

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.

edited Apr 07 '20 at 21:03

answered Apr 07 '20 at 20:02

sparik

1,181
9
16

Just remove the extra parentheses. I'll update my answer – sparik Apr 07 '20 at 20:45
actually there was a reason putting 2 parentheses as the compiler was interpreting as a function declaration. – Eduard Rostomyan Apr 07 '20 at 20:49
Right. Then you could use curly braces or copy initialization. – sparik Apr 07 '20 at 20:53
yeah I did already, although wasn't very clear for me why the first 2 arguments were discarded. Thank you – Eduard Rostomyan Apr 07 '20 at 20:54
Becuase of the comma operator. Basically, if you write `int a = (++x, y);`, then `++x` is evaluated and discarded, and `a` is equal to `y`. You can read more here https://en.cppreference.com/w/cpp/language/operator_other – sparik Apr 07 '20 at 21:01
I read that, but is there a rule under which compiler interprets as , operator? Like I am passing arguments to thread constructor, how come they can be discarded? – Eduard Rostomyan Apr 07 '20 at 21:03
Well, if I understand correctly, according to the standard, you are passing just one argument. – sparik Apr 07 '20 at 21:07

Synchronized call of functions using std::condition_variable

1 Answers1