Re: [PATCH v3 0/6] Add support for ISO C11 threads.h

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 21/03/2017 13:49, Joseph Myers wrote:
> On Tue, 21 Mar 2017, Adhemerval Zanella wrote:
> 
>>> Did you review those C11 DRs related to threads to make sure the 
>>> implementation behaves properly regarding issues raised in those DRs (and 
>>> that there are tests of this if applicable)?
>>>
>>> Current DR log: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2109.htm
>>>
>>
>> Thanks for the link, I will check this out. Do you know off the top of your 
>> head which ones would be applicable? 
> 
> It appears to be: 405 414 416 424 449 469 470 479 480 493.

It indeed seems to be one related to threads definitions and in general
C11 clarification seems to align with POSIX thread definition, which
simplifies its implementation.  However I see some problematic ones:
416 (tss_t destruction unspecified) and 493 (Mutex Initialization
Underspecified), so I am copying Tovarld's and Martin since they were the
one that raised most the DR.  It could be good if they is any impending
reason to avoid implement C11 thread over POSIX primitives based
on current standard.

Below some observation of the raised DRs related to current patch
proposal:

405: The mutex specification

"The C11 specification of mutexes is missing the total order over all the
 calls on a particular mutex."

My understanding is current implementation with POSIX primitives should be
enough to guarantee C11 total order over mutex (which is currently based on
acquire/release semanthics).


416: tss_t destruction unspecified

I could not find any impeding wording from the proposed technical corrigendum
that would prevent us to implement tss function on top of pthread key ones.


414: Typos in 6.27 Threads <threads.h> 

The typos does clarify one definition of mtx_plain and raises a questioning: 
how should we proceed if mtx_timedlock is used along with a mtx_plain mutex? 

My understanding is thrd_error should be returned, however current GLIBC 
implementation makes no distinction between PTHREAD_MUTEX_NORMAL and 
PTHREAD_MUTEX_TIMED_NP (and current C11 patch follows this semanthic).

It is feasible to do so, but it would require maybe an extra field in
mtx_t definition or a GNU-only thread type (maybe not intended to be
exposed).


424: underspecification of tss_t

These issues are covered under DR 416.


449: What is the value of TSS_DTOR_ITERATIONS for implementations with no maximum?

The proposed committee response was to intentionally do not define a value of 
TSS_DTOR_ITERATIONS, so my view is align with PTHREAD_DESTRUCTOR_ITERATIONS 
should be suffice (even though various architectures redefine 
_POSIX_THREAD_DESTRUCTOR_ITERATIONS to same value in local_lim.h).


469: lock ownership vs. thread termination

Defined as future.  However from last committee discussion it seems that
they are aiming for similar POSIX description.  So I am not sure if there
is any impending stardard definition to use underlying POSIX implementation.


470: mtx_trylock should be allowed to fail spuriously

It align with POSIX and thus with __pthread_mutex_trylock which uses a 
atomic_compare_and_exchange_bool_acq (similar to atomic_compare_exchange_weak).


479: unclear specification of mtx_trylock on non-recursive muteness

Defined as future. I am not aware of any issue trying to use POSIX
in underlying implementation.


480: cnd_wait and cnd_timewait should allow spurious wake-ups

Define as open, although my understanding is C11 definitions from last proposed
technical corrigendum may now allow spurious wakes ups (from the words "until
it is unblocked due to an unspecified reason"). So I think implementing
cnd_{timed}wait as pthread_cond_{timed}wait wrapper should be still valid.


493: Mutex Initialization Underspecified

Define as open and the proposed committee response follows:

  - Problems with mtx_init(): I think the only applicable point to curent
    patch is the last one:

    7. thrd_error shall be returned by mtx_init() when passed a NULL pointer.

    Which currently leads to a segfault (since it is based on pthread_mutex_init).
    It is simple to align current patch proposal to this specification.

  - Problems with mtx_destroy(): wording aligns with POSIX definition, so
    implementing mtx_destroy as a pthread_mutex_destroy wrapper should be ok.

  - Other Problems: wording aligns with POSIX definition as well, so I think
    there is no requirement to change for current patch.