condition_variable, mutex and flag in one struct

Question

Is to ok to combine all three variables into one struct?

struct lock_struct
{
    std::mutex mutex;
    std::conditional_variable cv;
    bool flag;
};

Are there any hidden synchronization problems with this approach? I do not intend to modify struct itself, only it's fields.

By the way, should I use bool or std::atomic<bool> when dealing with std::condition_variable's flag?

Edit: implemented it based on the answers.

class ConditionLock
{
public:
    void wait();
    void notify();
    bool getFlag() const;
private:
    mutable std::mutex _mutex;
    std::condition_variable _cv;
    bool flag;
};

void ConditionLock::wait()
{
    std::unique_lock<std::mutex> lock(_mutex);
    _cv.wait(lock, [&] { return flag; });
}

void ConditionLock::notify()
{
    std::unique_lock<std::mutex> lock(_mutex);
    flag = true;
    lock.unlock();
    _cv.notify_all();
}

bool ConditionLock::getFlag() const
{
    std::lock_guard<std::mutex> lock(_mutex);
    return flag;
}

I hope it's a correct implementation.

`bool` and `std::atmoic` have different semantics, you will have to show how you plan to use it — Passer By, Dec 04 '17 at 10:23
Your most significant risk is that you make the mutex public - meaning that anyone and their dog can lock it ... this puts you at risk of deadlocks; Putting the mutex and condition variable together though seems perfectly reasonable — UKMonkey, Dec 04 '17 at 10:26
" I do not intend to modify struct itself, only it's fields." Modifying a struct is modifying its members, thus its not clear what you mean by that — 463035818_is_not_an_ai, Dec 04 '17 at 10:30
If all uses of `flag` will be protected by the mutex, it's fine as `bool` as mutex locking and unlocking enforce the correct memory ordering. Otherwise… as PasserBy said the exact example is required to tell. — spectras, Dec 04 '17 at 11:42
@tobi303 I meant I would modify internal fields of a structure, but would not reassign the structure instance. I meant that, the way I see it, there would be no need to create a mutex to protect the structure instance. — CorellianAle, Dec 04 '17 at 13:05

score 6 · Accepted Answer · answered Dec 04 '17 at 11:45

Is to ok to combine all three variables into one struct?

Yes.

Are there any hidden synchronization problems with this approach?

Structure definition does not describe or enforce its intended usage. Since all the members are publicly accessible nothing prevents from incorrect or unintended usage.

A safer definition is make it a class with no public data members but public member functions.

By the way, should I use bool or std::atomic<bool> when dealing with std::condition_variable's flag

bool is enough as long as accessing that bool happens only when the mutex is locked. You can enforce this by making it a class with no public data members.

Note that if you make it std::atomic<bool> and modify it and signal the condition variable without locking the mutex, that leads to a race condition that causes the notifications from the condition variable to be lost, e.g.:

Thread 1             |  Thread 2
                     |  check the bool
modify the bool      |
signal the condition |  <notification not received>
                     |  wait on the codition

condition_variable, mutex and flag in one struct

1 Answers1