Is the "unique_lock& lock" parameter really unavoidable when using condition_variable with atomic_flag, or am I missing something?

Question

What I have here is an std::jthread to run some tasks every 10 seconds,
while allowing the thread to quit on signal during sleep, which would probably be something like this:

void ThreadMain(std::condition_variable& cv, const std::atomic_flag& stop)
{
    [[maybe_unused]] std::mutex mtx;
    [[maybe_unused]] std::unique_lock<std::mutex> lock(mtx);
    for(;;)
    {
        DoSomeTask();
        if(cv.wait_for(lock, 10s, [&stop]() {
            return stop.test();
        }))
        {
            return;
        }
    }
}

int main()
{
    std::condition_variable cv;
    std::atomic_flag stop;
    std::jthread thread(&ThreadMain, std::ref(cv), std::cref(stop));
    ...
}

Then somewhere in main do this:

stop.test_and_set();
cv.notify_all();

to stop the thread.

What I'm trying to ask here is that, since we're using std::atomic_flag, we don't need the unique_lock<mutex>& lock at all, but we still have to declare a dummy lock for condition_variable. (Last checked there isn't a std::atomic_flag::wait_for, as it would be perfect for this exact scenario.)
Am I missing something? Is this really just how it is under the current standard, or am I falling into an XY problem where there is a much simpler syntax for what I'm trying to achieve?
I'm probably just being pedantic though as an unused std::mutex most likely won't make any difference.

Edit: I just found out about jthread::request_stop and stop_token, so we don't need the atomic_flag, but we still need a condition_variable (and in turn an unused lock) for wait_for

"Last checked there isn't a std::atomic_flag::wait_for" : https://en.cppreference.com/w/cpp/atomic/atomic_flag/wait — 463035818_is_not_an_ai, Jun 21 '23 at 13:54
@463035818_is_not_an_ai It's not `..._for`, there's no duration parameter. — HolyBlackCat, Jun 21 '23 at 13:56
@HolyBlackCat um, its actually all there, I was just being inattentive — 463035818_is_not_an_ai, Jun 21 '23 at 13:57

score 1 · Accepted Answer · answered Jun 21 '23 at 16:21

The call

cv.wait_for(lock, 10s, stop_waiting)

is equivalent to

while (!stop_waiting()) {
    if (cv.wait_for(lock, 10s) == std::cv_status::timeout) {
        return stop_waiting();
    }
}
return true;

The following sequence is possible in your snippet:

notifying thread     waiting thread
----------------     --------------
                     lock the mutex
                     DoSomething()
                     evaluate stop_waiting() == false
set the flag
notify all
                     unlock the mutex and enter wait
                     wait for 10s

We have a lost notification. The waiting thread waits for 10s and times out. This pattern won't cause UB or a deadlock, but it will cause an avoidable delay in the notification. That can be problematic.

The idiomatic pattern would be to modify the state used by the predicate under the same mutex used by the wait:

// in main
std::mutex mtx
std::condition_variable cv;
bool stop = false;

{
    std::lock_guard guard{mtx}
    stop = true;
    cv.notify_all();
}

In this pattern, the predicate evaluation for waiting and the mutation that could change that evaluation both happen under the protection of the same lock.

Yeah, I just saw this on the cppreference page of std::condition_variable, "Even if the shared variable is atomic, it must be modified while owning the mutex to correctly publish the modification to the waiting thread." — Agritite, Jun 22 '23 at 12:49

Is the "unique_lock& lock" parameter really unavoidable when using condition_variable with atomic_flag, or am I missing something?

1 Answers1