On Wed, Sep 12, 2018 at 11:56 AM Mike Crowe <theopengr...@mac.mcrowe.com> wrote: > > 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.
The existing pthread_cond_timedwait() would still use CLOCK_REALTIME by default, so users would get to chose between those two clocks depending on the function that they used. (Of course not on Android though, given the workaround I described). > > [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. That's true; I am calling what we did a workaround for a reason, since in an ideal world we would offer the full functionality. However the situation is that libc++ either always waits on CLOCK_REALTIME or always waits on CLOCK_MONOTONIC after a conversion and the latter is preferable for us. I, too, haven't encountered a use case for waiting on a condition variable with CLOCK_REALTIME, even when analyzing all of the waiters in our system before submitting that workaround. > > > > > 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. Neither have I. The other function that I have seen used often and that suffers from this issue is sem_timedwait(). It doesn't have the option to use CLOCK_MONOTONIC during initialization either, so it is particularly dangerous to use on systems where CLOCK_REALTIME may jump. > > > > > 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 starting the conversation here. We've also been trying to fix this problem for years. > > Thanks for your feedback. > > Mike. Tom
