[PATCH] nptl: Provide a way to block all signals for the timer helper thread

[Adhemerval Zanella]

On 14/05/2020 16:33, Carlos O'Donell wrote:
> On 5/14/20 2:30 PM, Florian Weimer wrote:
>> * Carlos O'Donell:
>>
>>> On 5/12/20 3:17 PM, Florian Weimer wrote:
>>>> * Carlos O'Donell:
>>>>
>>>>>> +/* Exactly like pthread_create if NEW_SIGMASK == NULL.  Otherwise, do
>>>>>> +   not use the current signal mask for the new thread, but set it to
>>>>>> +   *NEW_SIGMASK instead (without unblocking internal signals).  */
>>>
>>>
>>> Suggest:
>>>
>>> /* Exactly like pthread_create if NEW_SIGMASK is NULL.
>>>    Create the new thread using the thread descriptor at NEWTHREAD,
>>>    and the thread attributes from *ATTR, executing provided START_ROUTINE
>>>    with a single void argument ARG.  The last argument is not present in
>>>    the public API for pthread_create, but is part of the internal glibc API.
>>>    The last argument is the signal mask to be restored in the new thread,
>>>    and is normally NULL, in which case the parent's signal mask is restored
>>>    as mandated by the standard's requirement for pthread_create.  In some cases
>>>    though you may wish to keep certain signals blocked to avoid race cases,
>>>    and today this includes the helper thread created by SIGEV_THREAD where all
>>>    signals are blocked except SIGCANCEL.  Thus the new_sigmask argument is
>>>    primarily intended for the creation of helper threads not user threads.  */
>>
>> Does this mean you think this interface is more generally useful?
> 
> It might. I haven't reviewed C11 threads, getaddrinfo_a or aio uses.

If we eventually decouple SIGCANCEL and SIGTIMER this interface will most
likely be removed as well.  Not sure if it would be better to add extra
comments to document the behavior of if the VCS itself is a better tool.

> 
> Do we need to judge that tod> 
> My opinion is that if a patch creates new internal API, then that API should
> receive documentation in the form of a comment.
> 
> My preference for such documentation is that it be task-oriented, that is that
> it should explain how the interface operates (what you documented), and what
> task you would use it for (the task-based aspect).
> 
> The alternative is adding a comment that says this internal API is not to be
> used by anyone except the two current caller. In which case you're off the
> hook for documenting the internal interface.... someone else will have to do
> that when they extend it again or want to use it a third time.
> 
>> I'm still trying to make sense of this request.
> Review: "How Developers Use API Documentation: An Observation Study"
> http://sigdoc.acm.org/wp-content/uploads/2019/01/CDQ18002_Meng_Steinhardt_Schubert.pdf
> See: "Present conceptual information integrated with related tasks."
> 
> The concept I think we should represent is that the signal mask here is to avoid
> races in internally created threads that do specific tasks. Future developers may
> find it useful, and leverage that to do other things. We tie a bunch of things
> together in one paragraph: race cases with signals, intended purpose, and current
> uses.
> 
> If you object on the groups of cost/value for such documentation, then I'm OK with
> that, but then you have to mark these internal APIs as off-limits in some way.
>