The clock used for waiting on a condition variable is a property of the wait, not the condition variable

After writing my previous blog post regarding libstdc++ and libc++ being prone to races when waiting on a condition variable, I proposed the addition of a pthread_cond_timedwaitonclock function on Austin Group mailing list. The Austin Group is responsible for the POSIX® standards. Some of the responses I received made me realise that it may not be completely clear why the clock being a property of the wait rather than of the condition variable itself is beneficial. Some response even suggested that the fault lie with the C++ standard and it should be fixed so that it can be implemented on top of POSIX as it stands today.

So, let's consider this simple program that uses POSIX threads directly from C:

#include <pthread.h>
#include <stdbool.h>
#include <stdio.h>
#include <unistd.h>

struct my_state {
    pthread_mutex_t protect_result;
    pthread_cond_t result_changed;
    bool result_ready;
    double result;
};

void *other_thread(void *state_in)
{
    struct my_state *state = state_in;

    /* Do some work */
    sleep(40);

    pthread_mutex_lock(&state->protect_result);
    state->result = 42.0;
    state->result_ready = 1;
    pthread_mutex_unlock(&state->protect_result);
    pthread_cond_broadcast(&state->result_changed);
    return NULL;
}

bool wait_for_result(struct my_state *state)
{
    struct timespec expiry;
    clock_gettime(CLOCK_MONOTONIC, &expiry);
    expiry.tv_sec += 42;

    pthread_mutex_lock(&state->protect_result);
    while (!state->result_ready) {
        if (pthread_cond_timedwait(&state->result_changed,
                                   &state->protect_result,
                                   &expiry)) {
            pthread_mutex_unlock(&state->protect_result);
            fprintf(stderr, "Timeout!\n");
            return false;
        }
    }

    pthread_mutex_unlock(&state->protect_result);
    return true;
}

int main()
{
    struct my_state state;
    pthread_t thread;
    state.result_ready = false;

    pthread_mutex_init(&state.protect_result, NULL);

    pthread_condattr_t attr;
    pthread_condattr_init(&attr);
    pthread_condattr_setclock(&attr, CLOCK_MONOTONIC);
    pthread_cond_init(&state.result_changed, &attr);

    if (pthread_create(&thread, NULL, other_thread, &state) != 0) {
        fprintf(stderr, "Failed to create thread\n");
        return 2;
    }

    if (wait_for_result(&state))
        printf("Result=%f\n", state.result);
    else
        fprintf(stderr, "Timeout!\n");

    pthread_join(thread, NULL);

    return 0;
}

Hopefully there's nothing particularly surprising here. Notice that it's necessary to pass CLOCK_MONOTONIC to clock_gettime in wait_for_result and to pthread_condattr_setclock in main. If the clocks passed in these two locations differ, or the whole pthread_condattr_t initialisation is omitted altogether (which means that the default of CLOCK_REALTIME would be used) then we'll end up passing timespecs measured against different clocks, and we'll probably be waiting for either a very long time or no time at all.

Now, let's consider an approximately-equivalent C++ program:

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

using namespace std::experimental;
using namespace std::chrono;
using namespace std;

class my_state {
    mutex protect_result;
    condition_variable result_changed;
    optional<double> result;

public:
    void other_thread()
    {
        // Do some work
        std::this_thread::sleep_for(seconds(40));

        unique_lock<mutex> lock(protect_result);
        result = 42.0;
        result_changed.notify_all();
    }

    optional<double> wait_for_result()
    {
        unique_lock<mutex> lock(protect_result);
        result_changed.wait_until(lock, steady_clock::now() + seconds(42), [this] {
                return result.has_value();
            });
        return result;
    }
};

int main()
{
    my_state state;
    auto t = std::thread([&state]{ state.other_thread(); });

    optional<double> result = state.wait_for_result();
    if (result.has_value()) {
        cout << *result << "\n";
        return 0;
    } else {
        cerr << "Timeout\n";
        return 1;
    }
}

Unfortunately this program isn't really a good example of the way this would be done in C++¹, but it serves as a simple way to show waiting on a condition variable.

The important part is that there is only one mention of a clock. The wait_for_result function does the equivalent of clock_gettime by calling std::chrono::steady_clock::now() which returns an instance of std::chrono::steady_lock::time_point. This is a distinct type that represents (in libstdc++ and libc++ at least) a time point measured against the CLOCK_MONOTONIC clock. The expiration time is set by adding a number of seconds. The std::condition_variable::wait_until method knows from the type of the timeout parameter which clock should be used for the wait. There's no longer any duplication.

If we knew that we had a highly-synchronised realtime clock and wanted to co-ordinate between machines then we could switch to using CLOCK_REALTIME by simply changing steady_clock to system_clock. No other changes are required.² The type of the parameter passed to std::condition_variable::wait_until will now be std::chrono::system_clock::time_point, so the implementation will know which clock to use.

Of course, in this example, the wait_for_result function would actually be better written as:

    optional<double> wait_for_result()
    {
        unique_lock<mutex> lock(protect_result);
        result_changed.wait_for(lock, seconds(42), [this] {
                return result.has_value();
            });
        return result;
    }

so there's no mention of the clock at all. std::condition_variable::wait_for turns into the equivalent call to wait_until using std::chrono::steady_clock.

If the clock to use is part of the type of the time point, then it's not possible to accidentally wait using the wrong clock. Library code can easily provide a wait interface that is agnostic to the actual clock that will be used, whilst providing the same guarantees. It's even possible to support user-defined clocks (although these clocks need to be converted to a supported clock when making system calls.)

If, as POSIX currently requires, the clock must be provided at the time of std::condition_variable construction, then it is still possible to provide some of this clock agnosticism. An optional clock template parameter could be added to the std::condition_variable type and used to pass an appropriate clock to pthread_condattr_setclock. In the implementation of std::condition_variable::wait_until, any clock that doesn't match must be converted to the clock specified during construction (which must be stored, since cannot be retrieved from the condition variable instance itself.) A condition variable created to use CLOCK_MONOTONIC can wait against a std::chrono::system_clock::time_point parameter, but any warping of the system clock during the wait will have no effect. A condition variable created to use CLOCK_REALTIME cannot wait against a std::chrono::steady_clock::time_point parameter without risking an incorrect wait time if the system clock is warped at the wrong moment.

In conclusion, there are real race conditions here for systems that may not always have access to a reliable source of real time. This is a particular problem just after boot. Adding appropriate functions to POSIX that can wait against CLOCK_MONOTONIC can solve these problems.

---

1. This situation is exactly what std::future is used for, so we'd be better off using std::async.↩

2. On Linux at least, CLOCK_MONOTONIC is affected by incremental time adjustments applied by adjtime anyway. However, if the system is using absolute timeouts across machines then CLOCK_REALTIME may still be an appropriate choice.↩