[18/19] manual: Add pthread_attr_setsigmask_np, pthread_attr_getsigmask_np

  And the PTHREAD_ATTR_NO_SIGMASK_NP constant.
---
 manual/threads.texi | 47 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)
  

Hi Florian,

On Tue, May 19, 2020 at 2:04 PM Florian Weimer via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> And the PTHREAD_ATTR_NO_SIGMASK_NP constant.
> ---
>  manual/threads.texi | 47 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>
> diff --git a/manual/threads.texi b/manual/threads.texi
> index a425635179..28d7cf2abd 100644
> --- a/manual/threads.texi
> +++ b/manual/threads.texi
> @@ -625,6 +625,7 @@ the standard.
>  @menu
>  * Default Thread Attributes::             Setting default attributes for
>                                           threads in a process.
> +* Initial Thread Signal Mask::            Setting the initial mask of threads.
>  * Waiting with Explicit Clocks::          Functions for waiting with an
>                                            explicit clock specification.
>  @end menu
> @@ -671,6 +672,52 @@ The system does not have sufficient memory.
>  @end table
>  @end deftypefun
>
> +@node Initial Thread Signal Mask
> +@subsubsection Controlling the Initial Signal Mask of a New Thread
> +
> +@Theglibc{} provides a way to specify the initial signal mask of a
> +thread created using @code{pthread_create}, passing a thread attribute
> +object configured for this purpose.
> +
> +These functions achieve the same effect as blocking all signals using
> +@code{pthread_sigmask} before creating the thread, and setting the
> +desired signal mask from the new thread, but in a more explicit
> +fashion.

So, I'm not quite clear what's going on here. Is it like this:

First thread

Block all signals
    |
    V
Create new thread --->NEW THREAD ---> Set mask to attr value
    |
    V
Restore signal mask to original value.

If so, maybe that could be explained a bit more explicitly?

> +
> +@deftypefun int pthread_attr_setsigmask_np (pthread_attr_t *@var{attr}, const sigset_t *@var{sigmask})
> +@standards{GNU, pthread.h}
> +@safety{@prelim{}@mtsafe{}@asunsafe{@ascuheap{}}@acunsafe{@acsmem{}}}
> +Change the initial signal mask specified by @var{attr}.  If
> +@var{sigmask} is not @code{NULL}, the initial signal mask for new
> +threads created with @var{attr} is set to @code{*@var{sigmask}}.  If
> +@var{sigmask} is @code{NULL}, @var{attr} will no longer specify an
> +explicit signal mask, so that the initial signal mask of the new
> +thread is inherited from the parent thread creating the new thread.

I don't like "parent" here. Threads are peers.

s/parent thread creating the new thread/creating thread/

> +
> +This function returns zero on success, and @code{ENOMEM} on memory
> +allocation failure.
> +@end deftypefun
> +
> +@deftypefun int pthread_attr_getsigmask_np (const pthread_attr_t *@var{attr}, sigset_t *@var{sigmask})
> +@standards{GNU, pthread.h}
> +@safety{@prelim{}@mtsafe{}@asunsafe{@ascuheap{}}@acunsafe{@acsmem{}}}
> +Retrieve the initial signal mask stored in @var{attr} and write it ot

s/initial// ?
s/write/save/ ?
s/ot/to/

> +@code{*@var{sigmask}}.  If the signal mask has not been set, return
> +the special constant @code{PTHREAD_ATTR_NO_SIGMASK_NP}, otherwise
> +return zero.
> +
> +Obtaining the signal mask only works if it has been previously stored
> +by @code{pthread_attr_setsigmask_np}.  For example, the
> +@code{pthread_getattr_np} function does not obtain the current signal
> +mask of the specified thread, and @code{pthread_attr_setsigmask_np}
> +will subsequently report the signal mask as unset.
> +@end deftypefun

I see what you're trying to say above, but maybe it would be clearer
to say something like: all that these APIs are doing is manipulating
an attribute setting in an object that is used to create new threads
and pthread_attr_getsigmask_np simply retrieves that attribute setting
(and never the calling thread's signal mask).

> +@deftypevr Macro int PTHREAD_ATTR_NO_SIGMASK_NP
> +The special value returned by @code{pthread_attr_setsigmask_np} to
> +indicate that no signal mask has been set for the attribute.
> +@end deftypevr
> +
>  @node Waiting with Explicit Clocks
>  @subsubsection Functions for Waiting According to a Specific Clock

Thanks,

Michael
  

On 5/19/20 6:45 AM, Florian Weimer via Libc-alpha wrote:
> And the PTHREAD_ATTR_NO_SIGMASK_NP constant.
> ---
>  manual/threads.texi | 47 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 

OK for master. Straight forward extension of the existing API and makes
it easier to set the signal mask for the created thread in the attribute.
The extension mechanism implemented for this should allow more flexible
changes in the future.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> diff --git a/manual/threads.texi b/manual/threads.texi
> index a425635179..28d7cf2abd 100644
> --- a/manual/threads.texi
> +++ b/manual/threads.texi
> @@ -625,6 +625,7 @@ the standard.
>  @menu
>  * Default Thread Attributes::             Setting default attributes for
>  					  threads in a process.
> +* Initial Thread Signal Mask::            Setting the initial mask of threads.
>  * Waiting with Explicit Clocks::          Functions for waiting with an
>                                            explicit clock specification.
>  @end menu
> @@ -671,6 +672,52 @@ The system does not have sufficient memory.
>  @end table
>  @end deftypefun
>  
> +@node Initial Thread Signal Mask
> +@subsubsection Controlling the Initial Signal Mask of a New Thread
> +
> +@Theglibc{} provides a way to specify the initial signal mask of a
> +thread created using @code{pthread_create}, passing a thread attribute
> +object configured for this purpose.
> +
> +These functions achieve the same effect as blocking all signals using
> +@code{pthread_sigmask} before creating the thread, and setting the
> +desired signal mask from the new thread, but in a more explicit
> +fashion.
> +
> +@deftypefun int pthread_attr_setsigmask_np (pthread_attr_t *@var{attr}, const sigset_t *@var{sigmask})
> +@standards{GNU, pthread.h}
> +@safety{@prelim{}@mtsafe{}@asunsafe{@ascuheap{}}@acunsafe{@acsmem{}}}
> +Change the initial signal mask specified by @var{attr}.  If
> +@var{sigmask} is not @code{NULL}, the initial signal mask for new
> +threads created with @var{attr} is set to @code{*@var{sigmask}}.  If
> +@var{sigmask} is @code{NULL}, @var{attr} will no longer specify an
> +explicit signal mask, so that the initial signal mask of the new
> +thread is inherited from the parent thread creating the new thread.

OK.

> +
> +This function returns zero on success, and @code{ENOMEM} on memory
> +allocation failure.

OK.

> +@end deftypefun
> +
> +@deftypefun int pthread_attr_getsigmask_np (const pthread_attr_t *@var{attr}, sigset_t *@var{sigmask})
> +@standards{GNU, pthread.h}
> +@safety{@prelim{}@mtsafe{}@asunsafe{@ascuheap{}}@acunsafe{@acsmem{}}}
> +Retrieve the initial signal mask stored in @var{attr} and write it ot
> +@code{*@var{sigmask}}.  If the signal mask has not been set, return
> +the special constant @code{PTHREAD_ATTR_NO_SIGMASK_NP}, otherwise
> +return zero.

OK.

> +
> +Obtaining the signal mask only works if it has been previously stored
> +by @code{pthread_attr_setsigmask_np}.  For example, the
> +@code{pthread_getattr_np} function does not obtain the current signal
> +mask of the specified thread, and @code{pthread_attr_setsigmask_np}
> +will subsequently report the signal mask as unset.
> +@end deftypefun
> +
> +@deftypevr Macro int PTHREAD_ATTR_NO_SIGMASK_NP
> +The special value returned by @code{pthread_attr_setsigmask_np} to
> +indicate that no signal mask has been set for the attribute.

OK.

> +@end deftypevr
> +
>  @node Waiting with Explicit Clocks
>  @subsubsection Functions for Waiting According to a Specific Clock
>  
>
  

* Michael Kerrisk:

> Hi Florian,
>
> On Tue, May 19, 2020 at 2:04 PM Florian Weimer via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> And the PTHREAD_ATTR_NO_SIGMASK_NP constant.
>> ---
>>  manual/threads.texi | 47 +++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 47 insertions(+)
>>
>> diff --git a/manual/threads.texi b/manual/threads.texi
>> index a425635179..28d7cf2abd 100644
>> --- a/manual/threads.texi
>> +++ b/manual/threads.texi
>> @@ -625,6 +625,7 @@ the standard.
>>  @menu
>>  * Default Thread Attributes::             Setting default attributes for
>>                                           threads in a process.
>> +* Initial Thread Signal Mask::            Setting the initial mask of threads.
>>  * Waiting with Explicit Clocks::          Functions for waiting with an
>>                                            explicit clock specification.
>>  @end menu
>> @@ -671,6 +672,52 @@ The system does not have sufficient memory.
>>  @end table
>>  @end deftypefun
>>
>> +@node Initial Thread Signal Mask
>> +@subsubsection Controlling the Initial Signal Mask of a New Thread
>> +
>> +@Theglibc{} provides a way to specify the initial signal mask of a
>> +thread created using @code{pthread_create}, passing a thread attribute
>> +object configured for this purpose.
>> +
>> +These functions achieve the same effect as blocking all signals using
>> +@code{pthread_sigmask} before creating the thread, and setting the
>> +desired signal mask from the new thread, but in a more explicit
>> +fashion.
>
> So, I'm not quite clear what's going on here. Is it like this:
>
> First thread
>
> Block all signals
>     |
>     V
> Create new thread --->NEW THREAD ---> Set mask to attr value
>     |
>     V
> Restore signal mask to original value.
>
> If so, maybe that could be explained a bit more explicitly?

I added this as a longer explanation to the end of the section and
dropped the second paragraph quoted above.

>> +@deftypefun int pthread_attr_setsigmask_np (pthread_attr_t *@var{attr}, const sigset_t *@var{sigmask})
>> +@standards{GNU, pthread.h}
>> +@safety{@prelim{}@mtsafe{}@asunsafe{@ascuheap{}}@acunsafe{@acsmem{}}}
>> +Change the initial signal mask specified by @var{attr}.  If
>> +@var{sigmask} is not @code{NULL}, the initial signal mask for new
>> +threads created with @var{attr} is set to @code{*@var{sigmask}}.  If
>> +@var{sigmask} is @code{NULL}, @var{attr} will no longer specify an
>> +explicit signal mask, so that the initial signal mask of the new
>> +thread is inherited from the parent thread creating the new thread.
>
> I don't like "parent" here. Threads are peers.
>
> s/parent thread creating the new thread/creating thread/

This is what I have now:

+@var{sigmask} is @code{NULL}, @var{attr} will no longer specify an
+explicit signal mask, so that the initial signal mask of the new
+thread is inherited from the thread that calls @code{pthread_create}.

>> +
>> +This function returns zero on success, and @code{ENOMEM} on memory
>> +allocation failure.
>> +@end deftypefun
>> +
>> +@deftypefun int pthread_attr_getsigmask_np (const pthread_attr_t *@var{attr}, sigset_t *@var{sigmask})
>> +@standards{GNU, pthread.h}
>> +@safety{@prelim{}@mtsafe{}@asunsafe{@ascuheap{}}@acunsafe{@acsmem{}}}
>> +Retrieve the initial signal mask stored in @var{attr} and write it ot
>
> s/initial// ?
> s/write/save/ ?
> s/ot/to/

Fixed:

+Retrieve the signal mask stored in @var{attr} and copy it to
+@code{*@var{sigmask}}.  If the signal mask has not been set, return

>> +@code{*@var{sigmask}}.  If the signal mask has not been set, return
>> +the special constant @code{PTHREAD_ATTR_NO_SIGMASK_NP}, otherwise
>> +return zero.
>> +
>> +Obtaining the signal mask only works if it has been previously stored
>> +by @code{pthread_attr_setsigmask_np}.  For example, the
>> +@code{pthread_getattr_np} function does not obtain the current signal
>> +mask of the specified thread, and @code{pthread_attr_setsigmask_np}
>> +will subsequently report the signal mask as unset.
>> +@end deftypefun
>
> I see what you're trying to say above, but maybe it would be clearer
> to say something like: all that these APIs are doing is manipulating
> an attribute setting in an object that is used to create new threads
> and pthread_attr_getsigmask_np simply retrieves that attribute setting
> (and never the calling thread's signal mask).

The problem is that this should be really in the documentation of
pthread_getattr_np, which is currently undocumented. 8-(

I'm going to add a comment in front of the paragraph:

+@c Move this to the documentation of pthread_getattr_np once it exists.

I also fixed the second reference to pthread_attr_setsigmask_np (should
have been pthread_attr_getsigmask_np).

We could parse /proc/TID/status in pthread_getattr_np, to obtain the
signal mask of a foreign thread, but that seems over the top to me.

I'm going to post a new patch shortly.

Thanks,
Florian