On Wednesday 12 September 2018 at 11:29:26 -0700, Tom Cherry wrote: > On Sun, Sep 9, 2018 at 8:28 AM Mike Crowe <theopengr...@mac.mcrowe.com> wrote: > > > > On Thursday 30 August 2018 at 21:10:48 +0100, Mike Crowe wrote: > > > C++11's std::condition_variable, std::timed_mutex and > > > std::recursive_timed_mutex support waiting with a timeout specified using > > > an arbitrary clock. It's common to use std::chrono::steady_clock (which > > > corresponds to CLOCK_MONOTONIC) or std::chrono::system_clock (which > > > corresponds to CLOCK_REALTIME) for these waits, and other clocks end up > > > being converted to one of those when performing the wait. > > > > > > Unfortunately, I don't believe that it's possible to implement these on > > > top > > > of the pthread equivalents correctly for std::chrono::steady_clock (i.e. > > > CLOCK_MONOTONIC) since: > > > > > > 1. pthread_mutex_timedlock only supports a time measured against > > > CLOCK_REALTIME. > > > > > > 2. The clock used for pthread_cond_timedwait must be specified when the > > > condition variable is created by pthread_cond_init. The clock to be > > > used > > > for future waits is not known at the point that the > > > std::condition_variable constructor is called. > > > > > > I'm most interested in the std::condition_variable case. since I have code > > > that wants to wait using std::chrono::steady_clock. > > > > > > There are a number of possible workarounds for these. I've described some > > > in a blog post[1] and in defect 0001164[2]. But, my favourite solution is > > > to introduce a variant of pthread_cond_timedwait that takes a an > > > additional > > > clockid_t parameter: > > > > > > int pthread_cond_timedwaitonclock(pthread_cond_t *cond, > > > pthread_mutex_t *mutex, > > > clockid_t clockid, > > > const struct timespec *abstimeout); > > > > > > I've proposed[3] an implementation of this function for glibc (as > > > pthread_cond_timedwaitonclock_np) and it was suggested that I ought to > > > raise it here. (This led me to enter defect 0001164, but since that > > > yielded > > > no response I've finally got round to writing this email too.) > > > > > > The equivalent for mutex would probably be: > > > > > > int pthread_mutex_timedlockonclock(pthread_mutex_t *mutex, > > > clockid_t clockid, > > > const struct timespec *abstimeout); > > > > > > but I've not yet made any attempt to implement it in glibc. > > > > > > Would making the C++ standard library implementable on top of POSIX be > > > considered a good enough reason to add such functions? > > > > > > Are these functions generic enough or perhaps too generic? Android had a > > > pthread_cond_timedwait_monotonic function for a while, but deprecated it > > > when they added support for pthread_condattr_setclock. > > We (libc team for Android) have observed this problem too and have > seen it cause issues on multiple occasions, enough times that we > actually submitted a workaround that always converts the input > timespec to CLOCK_MONOTONIC and waits with that clock[1]. Of course > this only ameliorates the problem, but does not completely solve it, > since there is still a period in time before the conversion to > CLOCK_MONOTONIC happens, that if CLOCK_REALTIME changes abruptly, > we're back to the same bad behavior as before. > > For this reason, we added pthread_cond_timedwait_monotonic_np() back > in the P release of Android[2] along with equivalent functions for the > rest of the timed waits. I've also pushed various patches to libc++, > one that builds upon these changes to our libc[3] and one that > implements std::condition_variable from scratch, borrowing the > implementation from our libc[4], however neither made progress. > > As for having a separate clock id parameter, we thought about that > during the review of the above changes, but we both thought that 1) > there's no strong reason to wait on any clock other than > CLOCK_MONOTONIC for these functions and 2) the underlying futex > implementation only allows CLOCK_REALTIME and CLOCK_MONOTONIC, so we'd > have to do conversions in userspace and inevitably wait on > CLOCK_MONOTONIC anyway, so it's best to be clear and only give > CLOCK_MONOTONIC as an option.
I had got the impression that people in the "enterprise" world wanted to continue to be able to wait on CLOCK_REALTIME. Maybe that's not as true as I thought it was. [snip] > > > An application as a whole is interested in real time or in clock > > > monotonic time. > > > > I don't believe that's true, especially when code is hidden away in > > libraries. A calendar application may wish to use CLOCK_REALTIME for events > > but CLOCK_MONOTONIC for notification timeouts. > > Realistically, there are better waiting mechanisms than condition > variables that a calender application should use when waiting for > events. We have broken this use case if anyone is trying to do it on > Android given the above work around, but we believe the benefits > outweigh the risk someone is attempting to do something like this. Given the fact that the Linux kernel appears to behave perfectly when the clock is changed during CLOCK_REALTIME waits, and an application cannot do so by itself, it seems a shame to stop applications taking advantage of that. However, at least based on my experience, I wouldn't have a use for waiting on a condition variable using CLOCK_REALTIME. > > > pthread_mutex_timedlockonclock can be implemented using a condition > > > variable configured to use an appropriate clock. > > > > It can be, if pthread_cond_timedwaitonclock is available, otherwise you're > > back to not knowing the clock at construction time. I'm sceptical that > > adding the overhead of a condition variable for every std::timed_mutex > > instance will be acceptable to C++ standard library authors. > > Note that std::timed_mutex in libc++ is actually implemented as a > combination of a std::mutex and a std::condition_variable currently. Oh, it does in libstdc++ too. Whoops! I did say that I was more interested in condition variables than mutexes earlier. :) It looks like, although it might be useful for someone, there isn't any need to introduce a pthread_mutex_timedwait equivalent for CLOCK_MONOTONIC for the time being then. I've never found myself needing to use std::timed_mutex. > > > Non-monotonic changes to CLOCK_REALTIME should be rare since methods > > > exist to correct a slightly-wrong clock without it going backwards. > > > > Systems, particularly embedded ones, may boot without knowing the current > > time and only set the system clock later. This may be quite a long time > > later if they don't immediately have an Internet connection. A desktop user > > may notice that their clock is completely wrong and take action to correct > > it. > > We saw this happen on Android in real life use cases often enough that > we had multiple bugs filed against it, which lead us to the above > workaround that always uses CLOCK_MONOTONIC, regardless of what clock > the user selected, so unfortunately it is a real issue. I received an email today as a result of one of my previous libstdc++ patches from someone else who was running into this problem. It does seem to be quite common, I've been sporadically trying to solve it for several years and I'd really like to work out how best to get it fixed. Thanks for your feedback. Mike.
