Austin Group Defect Tracker

Aardvark Mark III


Viewing Issue Simple Details Jump to Notes ] Issue History ] Print ]
ID Category Severity Type Date Submitted Last Update
0001216 [1003.1(2016)/Issue7+TC2] System Interfaces Comment Enhancement Request 2018-11-26 18:53 2019-03-06 13:17
Reporter mikecrowe View Status public  
Assigned To ajosey
Priority normal Resolution Open  
Status Under Review  
Name Mike Crowe
Organization
User Reference
Section pthread
Page Number 0
Line Number 0
Interp Status ---
Final Accepted Text
Summary 0001216: Adding clockid parameter to functions that accept absolute struct timespec timeouts
Description POSIX contains several functions that support waiting with an absolute
timeout passed as a struct timespec. This time must almost always be
measured against CLOCK_REALTIME. (pthread_cond_timedwait also supports a
single alternative clock specified at construction time via
pthread_condattr_setclock.)

Embedded systems and desktop computers may not have a good source of
accurate time, particularly at boot. This can result in CLOCK_REALTIME
warping by a large amount when the real time is known. In such situations,
CLOCK_REALTIME is not a good choice for expressing timeouts. A member of
the Android libc team has reported[1] that this has been the cause of real
world bugs in Android applications. I've worked on software at different
companies where we had to work around this problem.

The C++ standard provides std::condition_variable::wait_until and
std::timed_mutex::try_lock_until methods which support arbitrary clocks.
Current implementations that build upon POSIX convert these clocks to
CLOCK_REALTIME, which can cause race conditions when CLOCK_REALTIME is
warped. The C++ standard requires the clock to be specified at the time of
the wait, which means that pthread_condattr_setclock isn't useful.

The above problems can be solved by adding variants of the affected
functions that take an extra clockid_t parameter to indicate the clock that
should be used. Initially, implementations would be required to only
support passing CLOCK_REALTIME which would make adding support
straightforward. Support for CLOCK_MONOTONIC would be suggested, and
implementations would be free to support other clocks if they wished.

This proposal is the result of a thread[2] on the mailing list and my
original defect report[3] only covering pthread_cond_timedwait.

Various naming options for the new functions were discussed[4] and the
following names are based on one of the more popular options. In all cases
the clock immediately precedes the timespec timeout.

int
pthread_mutex_clocklock(
    pthread_mutex_t *restrict mutex,
    clockid_t clock,
    const struct timespec *restrict abstime)

int
pthread_rwlock_clockrdlock(
    pthread_rwlock_t *restrict rwlock,
    clockid_t clock,
    const struct timespec *restrict abstime)

int
pthread_rwlock_clockwrlock(
    pthread_rwlock_t *restrict rwlock,
    clockid_t clock,
    const struct timespec *restrict abstime)

int
pthread_cond_clockwait(
    pthread_cond_t *restrict cond,
    pthread_mutex_t *restrict mutex,
    clockid_t clock,
    const struct timespec *restrict abstime)

int
sem_clockwait(
    sem_t *restrict sem,
    clockid_t clock,
    const struct timespec *restrict abstime)

ssize_t
mq_clockreceive(
    mqd_t mqdes, char *restrict msg_ptr,
    size_t msg_len,
    unsigned int *restrict msg_prio,
    clockid_t clock,
    const struct timespec *restrict abs_timeout)

int
mq_clocksend(
    mqd_t mqdes, const char *restrict msg_ptr,
    size_t msg_len, unsigned int msg_prio,
    clockid_t clock,
    const struct timespec *restrict abs_timeout)

These functions all behave the same as their "timed" equivalents, but
measure the timeout against the specified clock rather than CLOCK_REALTIME.

If passed an unsupported clock, these functions indicate failure in the
same way as their "timed" equivalents and return/set errno to ENOTSUP as
required.

Support for a clock by one function does not require that the clock be
supported by any of the others.

[1] www.mail-archive.com/austin-group-l@opengroup.org/msg02902.html">https://www.mail-archive.com/austin-group-l@opengroup.org/msg02902.html [www.mail-archive.com/austin-group-l@opengroup.org/msg02902.html" target="_blank">^]
[2] www.mail-archive.com/austin-group-l@opengroup.org/msg02813.html">https://www.mail-archive.com/austin-group-l@opengroup.org/msg02813.html [www.mail-archive.com/austin-group-l@opengroup.org/msg02813.html" target="_blank">^]
[3] http://austingroupbugs.net/view.php?id=1164 [^]
[4] www.mail-archive.com/austin-group-l@opengroup.org/msg03034.html">https://www.mail-archive.com/austin-group-l@opengroup.org/msg03034.html [www.mail-archive.com/austin-group-l@opengroup.org/msg03034.html" target="_blank">^]
Desired Action The addition of the above functions, or ones that provide equivalent functionality.
Tags No tags attached.
Attached Files

- Relationships
related to 0001164Closed Correct C++11 std::condition_variable requires a version of pthread_cond_timedwait that supports specifying the clock 

-  Notes
(0004171)
nick (manager)
2018-11-27 16:02

As a reminder, the Austin Group procedures (see https://opengroup.org/austin/docs/austin_sd6.txt) [^] state:

New Work Items


From time to time, a defect report may propose new work
items that are outside the scope of maintenance of the Austin Group
specifications. This section addresses how these are handled.

The Austin Group is not a development body for new material apart from
integration issues arising from the merger of the approved standards
that were the Base documents into the revision.

The Austin Group expects to take a similar approach for a future revision.
Thus if a defect report raises the possibility of new interfaces
for inclusion, the standard response will be that it is out of scope
for either a TC or Interpretation, and that if the new material were
to meet the some criteria it may be considered for inclusion in a
future revision subject to the agreed scope determined at that time,
although there is no guarantee.

The recommended criteria for development of new interfaces to enable
them to be considered for inclusion in a future revision are as follows:

1.There must be a written specification that has undergone a formal
consensus based approval process and is suitable for inclusion.

Parties interested in submitting new work items through one of the
three organizations within the Austin Group (The Open Group, IEEE, ISO/IEC)
should contact the appropriate Organizational Representative for further
information and advice on how each organization handles new work items.
Submissions from other organizations will also be considered.
Items 2 through 4 below apply to all submissions regardless of
origin.

2.There must be an implementation, preferably a reference implementation.

3.The specification must be "sponsored" by one of three organizations
(The Open Group, IEEE, ISO/IEC) within the Austin Group, i.e. they would
support and champion its inclusion.

4.Submitters must provide an outline plan of the editing instructions to
merge the document with the Austin Group specifications, and assistance
to the Austin Group editors as required to complete the merger.
(0004196)
mikecrowe (reporter)
2019-01-07 21:26

This issue was discussed in the 2018-11-29 teleconference[1] and I received
advice on how to proceed.

In particular, during the call we discussed making the new functions
distinguish between valid-but-unsupported clocks (such as
CLOCK_PROCESS_CPUTIME_ID) and an entirely invalid clock. After further
thought, I've realised that there are actually three separate error cases:

1. Valid, but nonsensical clock (such as CLOCK_PROCESS_CPUTIME_ID.)

2. Valid, but (currently) unsupported clock (such as CLOCK_MONOTONIC in an
   implementation that doesn't support it yet.)

3. Invalid clock ID.

Whilst implementing the new functions within glibc I realised that
returning different errors for these situations is error-prone because a
complete list of valid clocks is required for the target platform. There is
a high risk that a new clock is added, but not all the functions are
updated to return an appropriate error.

Therefore, I propose that all three cases return/set errno to EINVAL.

[1] www.mail-archive.com/austin-group-l@opengroup.org/msg03074.html">https://www.mail-archive.com/austin-group-l@opengroup.org/msg03074.html [www.mail-archive.com/austin-group-l@opengroup.org/msg03074.html" target="_blank">^]
(0004215)
mikecrowe (reporter)
2019-01-19 04:08

Here's my suggested wording for pthread_cond_clockwait (with a single hint
to the other new functions too.) It would be great to receive feedback on
whether the changes are broadly appropriate, and whether there is a better
way to describe the changes. Once I know whether I'm on the right track, I
can add the remaining functions (which will hopefully require fewer
changes.)

(I have glibc implementations of sem_clockwait, pthread_mutex_clockwait,
pthread_cond_clockwait, pthread_rwlock_clockrdlock and
pthread_rwlock_clockwrlock working. I just need to get the commit messages
into shape before posting them to the glibc list.)

Thanks.

Mike.

*** Add to lists at lines 3070, 10833, 18119, 52281, 52599, 52710, 55292, 124395, 130171:
pthread_cond_clockwait()

*** After line 10727 insert:
int pthread_cond_clockwait(pthread_cond_t *restrict,
    pthread_mutex_t *restrict, clockid_t,
    const struct timespec *restrict);

*** After line 10767 insert:
It calls pthread_cond_clockwait() with m as the mutex argument and the call returns zero or
certain error numbers (see pthread_cond_clockwait()).

*** After line 17972 insert:
It blocks in a call to pthread_cond_clockwait() with m as the mutex
argument.

*** Line 30449
Change "and pthread_cond_timedwait" to ", pthread_cond_timedwait and
pthread_cond_clockwait".

*** Lines 52513, 52515, 52519, 52582, 124488
Change "or pthread_cond_timedwait" to ", pthread_cond_timedwait or
pthread_cond_clockwait".

*** After line 52743 insert:
int pthread_cond_clockwait(pthread_cond_t *restrict cond,
    pthread_mutex_t *restrict mutex,
    clockid_t clock_id,
    const struct timespec *restrict abstime);

*** Lines 52747, 52762, 52763, 52766, 52781, 52783, 52786, 52848, 52882
Add ", pthread_cond_clockwait()" after "pthread_cond_timedwait()"

*** After line 52797 insert:
The pthread_cond_clockwait() function shall be equivalent to
pthread_cond_timedwait(), except that the absolute time specified by
abstime is measured against the clock indicated by clock_id rather than the
clock specified in the condition variable's clock attribute. All
implementations shall support passing CLOCK_REALTIME to
pthread_cond_clockwait() as the clock_id parameter. If the Monotonic Clock
option is supported, all implementations shall support a clock_id of
CLOCK_MONOTONIC.

*** Line 52822:
Change to "The pthread_cond_timedwait() and pthread_cond_clockwait()
functions shall fail if:"

*** Line 52823, 52850:
Change "pthread_cond_timedwait()" to "pthread_cond_timedwait() or
pthread_cond_clockwait()"

*** Line 52825:
Add "or the clock_id parameter passed to pthread_cond_clockwait is invalid
or not supported."

*** Insert after line 52879:
Choice of clock

Care should be taken to decide which clock is most appropriate when waiting
with a timeout. The system clock, CLOCK_REALTIME, as used by default with
pthread_cond_timedwait() may be subject to jumps forwards and backwards in
order to correct it against actual time. CLOCK_MONOTONIC is guaranteed not
to jump backwards and must also advance in real time, so using it via
pthread_cond_clockwait() or pthread_condattr_setclock() may be
more appropriate.

*** Line 52943:
Replace CLOCK_REALTIME with CLOCK_MONOTONIC

*** Line 52947:
Replace line with:
 rc = pthread_cond_clockwait(&t.cond, &t.mn, CLOCK_MONOTONIC, &ts);

*** After line line 52956 insert:
Using CLOCK_MONOTONIC rather than CLOCK_REALTIME means that the timeout is
not influenced by the system clock being changed.

*** After line 53084 insert:
Note that the /clock/ attribute shall have no effect on the
pthread_cond_clockwait() function.

*** Lines 54147, 54186, 54321, 55282:
After "pthread_cond_timedwait()" insert ", pthread_cond_clockwait()".

*** Line 123502:
Delete remainder of paragraph starting with "This capability has not been
added to other functions...". Insert in its place:

For condition variables, this capability is also available by passing
CLOCK_MONOTONIC to the pthread_cond_clockwait() function. Similarly,
CLOCK_MONOTONIC can be specified when calling sem_clockwait,
pthread_mutex_clocklock, pthread_rwlock_clockrdlock and
pthread_rwlock_clockwrlock.

*** After line 123535:
Subsequently, it was discovered that using a particular clock for the
timeout was desired by some users. In particular, the ISO C++11 standard
requires the clock to be a property of the wait for its
std::condition_variable and std::timed_mutex. This led to the addition of
sem_clockwait(), pthread_mutex_clockwait(), pthread_cond_clockwait(),
pthread_rwlock_clockrdlock() and pthread_rwlock_clockwrlock() functions
that accepted a clock parameter to measure their absolute timeout against.
The addition of the corresponding mq_clockreceive and mq_clocksend may be
considered in the future.
(0004276)
mikecrowe (reporter)
2019-03-06 13:17

I've posted patches for pthread_cond_clockwait, sem_clockwait, pthread_rwlock_clockrdlock and pthread_rwlock_clockwrlock to the glibc mailing list. I received only one reply for a straightforward-to-fix problem.

https://sourceware.org/ml/libc-alpha/2019-02/msg00637.html [^]

pthread_mutex_clocklock requires more work.

- Issue History
Date Modified Username Field Change
2018-11-26 18:53 mikecrowe New Issue
2018-11-26 18:53 mikecrowe Status New => Under Review
2018-11-26 18:53 mikecrowe Assigned To => ajosey
2018-11-26 18:53 mikecrowe Name => Mike Crowe
2018-11-26 18:53 mikecrowe Section => pthread
2018-11-26 18:53 mikecrowe Page Number => 0
2018-11-26 18:53 mikecrowe Line Number => 0
2018-11-27 09:23 geoffclare Project 1003.1(2008)/Issue 7 => 1003.1(2016)/Issue7+TC2
2018-11-27 09:23 geoffclare Relationship added related to 0001164
2018-11-27 16:02 nick Note Added: 0004171
2019-01-07 21:26 mikecrowe Note Added: 0004196
2019-01-19 04:08 mikecrowe Note Added: 0004215
2019-03-06 13:17 mikecrowe Note Added: 0004276


Mantis 1.1.6[^]
Copyright © 2000 - 2008 Mantis Group
Powered by Mantis Bugtracker