libcxx std::condition_variable::wait_for() fix for when tick occurs during wait_for()

[bcain]

Summary

During std::condition_variable::wait_for(), we sample the system clock and then we sample the steady clock.

If a tick occurs after the system clock is sampled, but prior to when the steady clock is sampled, then the __do_timed_wait() may elapse the entire duration but the check at the end will fail and we'll return cv_status::no_timeout in error. In the thread.condition.condvar/wait_for test, this causes the test to fail for my hexagon target.

§ 30.5.1 states “Returns: cv_status::timeout if the relative timeout (30.2.4) specified by rel_time expired, otherwise cv_status::no_timeout.”

This proposed fix would sample the steady clock first. Now, if the tick elapses between the two the check would yield the same results as the underlying __do_timed_wait().

[EricWF]

I would love to know what platform you ran into this on.

[EricWF]

Also I would love to see an attempt at writing a test for this.

[bcain]

In D27715#631540, @EricWF wrote:

Also I would love to see an attempt at writing a test for this.

This was encountered on Hexagon. thread.condition.condvar/wait_for detects the failure, so to some extent a test already exists. I suspect that linux x86_64 would see the failure on that test if CONFIG_HZ were specified to be extremely high (> 40000).

An isolated, reliably failing test seems difficult/impossible since it would require the test case to simulate a system that knows quite a bit about the implementation ("system clock was just sampled in the wait_for context, so let's trigger a tick interrupt now"). A test case like this could be constructed but it would be a real challenge to integrate into the existing lit test suite.

[bcain]

this was committed by Marshall

include/__mutex_base

condition_variable::wait_for(unique_lock<mutex>& __lk,
                              const chrono::duration<_Rep, _Period>& __d)
 {
     using namespace chrono;
     if (__d <= __d.zero())
         return cv_status::timeout;
     typedef time_point<system_clock, duration<long double, nano> > __sys_tpf;
     typedef time_point<system_clock, nanoseconds> __sys_tpi;
     __sys_tpf _Max = __sys_tpi::max();
-    system_clock::time_point __s_now = system_clock::now();
+
+    // Steady clock must be sampled prior to system clock
     steady_clock::time_point __c_now = steady_clock::now();
+    system_clock::time_point __s_now = system_clock::now();
     if (_Max - __d > __s_now)
         __do_timed_wait(__lk, __s_now + __ceil<nanoseconds>(__d));
     else
         __do_timed_wait(__lk, __sys_tpi::max());
     return steady_clock::now() - __c_now < __d ? cv_status::no_timeout :
                                                  cv_status::timeout;
 }