Re: [MTASCsft PATCH WIP6 01/33] MT-, AS- and AC-Safety docs

["Carlos O'Donell" <carlos at redhat dot com>]

On 01/22/2014 02:41 PM, Alexandre Oliva wrote:
> On Jan 22, 2014, "Carlos O'Donell" <carlos@redhat.com> wrote:
> 
>> > Excellent use of the noun form `work-around'; do you have a preference
>> > for `work-around' vs `workaround?' Both forms are correct, but I think
>> > we may wish to use `workaround' in the manual since it's one less
>> > character and perhaps more common in our technical field.
> The following incremental patch fixes that, in addition to a few other
> changes I made in intro.texi and macros.texi.
> 
> I've combined @mtasutimer and @acutimer into @mtascutimer, and @mtasusig
> and @acusig into @mtascusig, upon realizing that they were always
> together and there's no point in keeping them separate, since they will
> always be together.  @mtstimer was added for functions that *only* set
> the timer (alarm, setitimer), so they are safe in what they do, but
> combining them may become problematic.
> 
> I introduced @mtshostid, used in gethostid and sethostid previously
> without a definition.  I script I wrote to catch odd safety remarks
> reminded me I had to define it.  I'm including it in a separate future
> patch.
> 
> I also reworded the definition of race per our IRC conversation the
> other day.
> 
> Please let me know whether you'd prefer me to keep on posting
> incremental changes and consolidating them into a
> to-be-posted-consolidated patch#1, or if I should keep the incremental
> patches for separate installation.

Thanks this is looking almost done.

> diff --git a/manual/intro.texi b/manual/intro.texi
> index fede2fd..25cef22a 100644
> --- a/manual/intro.texi
> +++ b/manual/intro.texi
> @@ -163,7 +163,7 @@ pattern matching facilities (@pxref{Pattern Matching}).
>  * POSIX Safety Concepts::       Safety concepts from POSIX.
>  * Unsafe Features::             Features that make functions unsafe.
>  * Conditionally Safe Features:: Features that make functions unsafe
> -                                 in the absence of work-arounds.
> +                                 in the absence of workarounds.

OK.

>  * Other Safety Remarks::        Additional safety features and remarks.
>  @end menu
>  
> @@ -438,8 +438,11 @@ Functions marked with @code{timer} use the @code{alarm} function or
>  similar to set a time-out for a system call or a long-running operation.
>  In a multi-threaded program, there is a risk that the time-out signal
>  will be delivered to a different thread, thus failing to interrupt the
> -intended thread.
> -
> +intended thread.  Besides being MT-Unsafe, such functions are always
> +AS-Unsafe, because calling them in signal handlers may interfere with
> +timers set in the interrupted code, and AC-Unsafe, because there is no
> +safe way to guarantee an earlier timer will be reset in case of
> +asynchronous cancellation.

OK.
  
>  @end itemize
>  
> @@ -505,23 +508,49 @@ the objects are passed to the functions by users; in others, they are
>  used by the functions to return values to users; in others, they are not
>  even exposed to users.
>  
> -When the objects are passed by users, in some cases there might be
> -uncertainty as to whether the library takes care of synchronization on
> -its own, or whether callers are responsible for it.  We draw the line
> -for objects passed by users as follows: objects whose types are exposed
> -to callers, and that callers are expected to access directly, such as
> -memory buffers, strings, and various non-opaque structs, are @emph{not}
> -annotated with @code{race}, because it would be noisy and redundant with
> -the general requirement that users guard against data races.  As for
> -objects that are only to be manipulated by passing them to library
> -functions, there might be an expectation that the library will take care
> -of synchronization on its own, as it does for e.g. @code{FILE} streams;
> -when this expectation is not met because synchronization is entirely up
> -to callers, for example in the case of @code{obstacks}, we will annotate
> -functions that take such arguments with @code{race}, followed by a colon
> -and the argument name.  We will not, however, regard such functions as
> -MT-Unsafe for this reason; the notion that users are expected to
> -safeguard from data races objects under their control prevail.
> +We consider that protecting from data races objects passed as (indirect)
> +arguments to functions are caller's responsibility; we will not regard a
> +function as MT-Unsafe or AS-Unsafe if it misbehaves when users fail to
> +take the measures required by POSIX to avoid data races when dealing
> +with such objects.  

Suggest first sentence reorganization to:

We consider access to objects passed as (indirect) arguments to functions to
be data race free. The assurance of data race free objects is the caller's
responsibility. We will not mark a function as MT-Unsafe or AS-Unsafe if it 
misbehaves when users fail to take the measures required by POSIX to avoid 
data races when dealing with such objects.

>                      As a general rule, if a function is documented as
> +reading from an object passed (by reference) to it, or modifying it,
> +users ought to use memory synchronization primitives to avoid data races
> +just as they would should they perform the accesses themselves rather
> +than by calling the library function.  @code{FILE} streams are the
> +exception to the general rule, in that POSIX mandates the library to
> +guard against data races in many functions that manipulate objects of
> +this specific opaque type.  We regard this as a convenience provided to
> +users, rather than as a general requirement whose expectations should
> +extend to other types.

OK.

> +
> +In order to remind users that guarding certain arguments is their
> +responsibility, we will annotate functions that take objects of certain
> +types as arguments.  We draw the line for objects passed by users as
> +follows: objects whose types are exposed to users, and that users are
> +expected to access directly, such as memory buffers, strings, and
> +various user-visible @code{struct} types, do @emph{not} give reason for
> +functions to be annotated with @code{race}.  It would be noisy and
> +redundant with the general requirement, and not many would be surprised
> +by the library's lack of internal guards when accessing objects that can
> +be accessed directly by users.

OK.

> +
> +As for objects that are opaque or opaque-like, in that they are to be
> +manipulated only by passing them to library functions (e.g.,
> +@code{FILE}, @code{DIR}, @code{obstack}, @code{iconv_t}), there might be
> +additional expectations as to internal coordination of access by the
> +library.  We will annotate, with @code{race} followed by a colon and the
> +argument name, functions that take such objects but that do not take
> +care of synchronizing access to them by default.  For example,
> +@code{FILE} stream @code{unlocked} functions will be annotated, but
> +those that perform implicit locking on @code{FILE} streams by default
> +will not, even though the implicit locking may be disabled on a
> +per-stream basis.

OK.

> +In either case, we will not regard as MT-Unsafe functions that may
> +access user-supplied objects in unsafe ways should users fail to ensure
> +the accesses are well defined; 

s/;/./g

                                  the notion that users are expected to

s/the/  The/g

> +safeguard from data races any objects of their choice that the library
> +accesses on their behalf prevail.

s/prevail/prevails/g

Note: It's actually rather complicated to determine if prevail or prevails
should be used here. Common forms of modal and an infinitive are:
"should ... prevail" or "if it ... prevails" or in your case 
"the notion ... prevails."

>  @c The above describes @mtsrace; @mtasurace is described below.
>  
> @@ -615,10 +644,9 @@ non-recursive mutex while calling all functions that use the same
>  temporary signal; blocking that signal before the call and resetting its
>  handler afterwards is recommended.
>  
> -In functions marked with @code{sig} also as an AC-Safety issue, the
> -problem is more complex: the temporarily-installed signal handler may
> -remain configured and enabled for the whole process if the thread is
> -cancelled.
> +There is no safe way to guarantee the original signal handler is
> +restored in case of asynchronous cancellation, therefore so-marked
> +functions are also AC-Unsafe.

OK.

>  @c fixme: at least deferred cancellation should get it right, and would
>  @c obviate the restoring bit below, and the qualifier above.
> @@ -723,6 +751,20 @@ signals are enabled, and so the environment can be considered
>  effectively constant in these contexts, which makes the former safe.
>  
>  
> +@item @code{hostid}
> +@cindex hostid
> +
> +The function marked with @code{hostid} as an MT-Safety issue reads from
> +the system-wide data structures that hold the ``host ID'' of the
> +machine.  These data structures cannot generally be modified atomically.
> +Since it is expected that the ``host ID'' will not normally change, the
> +function that reads from it (@code{gethostid}) is regarded as safe,
> +whereas the function that modifies it (@code{sethostid}) is marked with
> +@code{@mtasuconst{:@mtshostid{}}}, indicating it may require special
> +care if it is to be called.  In this specific case, the special care
> +amounts to system-wide (not merely intra-process) coordination.

OK.

> +
> +
>  @item @code{sigintr}
>  @cindex sigintr
>  
> diff --git a/manual/macros.texi b/manual/macros.texi
> index ac691b8..11c69c7 100644
> --- a/manual/macros.texi
> +++ b/manual/macros.texi
> @@ -121,6 +121,9 @@ locale\comments\
>  @macro mtsenv {comments}
>  env\comments\
>  @end macro
> +@macro mtshostid {comments}
> +hostid\comments\
> +@end macro

OK.

>  @macro mtssigintr {comments}
>  sigintr\comments\
>  @end macro
> @@ -163,10 +166,7 @@ fd\comments\
>  @macro acsmem {comments}
>  mem\comments\
>  @end macro
> -@macro mtasusig {comments}
> -sig\comments\
> -@end macro
> -@macro acusig {comments}
> +@macro mtascusig {comments}

OK.

>  sig\comments\
>  @end macro
>  @macro mtasuterm {comments}
> @@ -175,7 +175,10 @@ term\comments\
>  @macro acuterm {comments}
>  term\comments\
>  @end macro
> -@macro mtutimer {comments}
> +@macro mtstimer {comments}
> +timer\comments\
> +@end macro
> +@macro mtascutimer {comments}

OK.

>  timer\comments\
>  @end macro
>  @macro mtasscwd {comments}

Cheers,
Carlos.