[Bug nptl/25158] warning when using non-thread-safe functions

["carlos at redhat dot com" <sourceware-bugzilla at sourceware dot org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=25158

--- Comment #9 from Carlos O'Donell <carlos at redhat dot com> ---
(In reply to Florian Weimer from comment #8)
> (In reply to Carlos O'Donell from comment #7)
> >> There is also an unresolved ambiguity regarding what thread safety means for
> >> function that updates objects whose address has been passed as parameters.
> > 
> > Can you give an exmaple of such a function? Are we talking about APIs like
> > strtok?
> 
> I think “MT-Safe race:obstack-ptr” is such an example.

Perfect. Good example. Where do you see the ambiguity?

Have you had a chance to read all of `info libc 'Conditionally Safe Features'`?

Particularly the "race" section?

Quoting from the manual:
~~~
     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.,
     ‘FILE’, ‘DIR’, ‘obstack’, ‘iconv_t’), there might be additional
     expectations as to internal coordination of access by the library.
     We will annotate, with ‘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, ‘FILE’
     stream ‘unlocked’ functions will be annotated, but those that
     perform implicit locking on ‘FILE’ streams by default will not,
     even though the implicit locking may be disabled on a per-stream
     basis.
~~~
obstack is not synchronized so we add race:obstack-ptr.

To some degree this is a concession to the legacy obstack ABI since it doesn't,
AFAIK, have room to add a local mutex for protection (the type is exposed
directly to users in obstack.h and is ABI).

However, in some cases there isn't an object to use. Consider two memcpy's from
two different threads to the same memory. memcpy is *always* MT-safe, and the
pointer to pointed memory cannot be meaningfully synchronized.

If we had a lock in obstack then we could just mark it MT-safe with no
notation.

In summary
- obstack is MT-safe so long as you avoid data races with the objects in
question, or following the POSIX model you synchronize memory accesses.

> >> In the past, some of functions which only operate on their arguments have
> >> been considered as not thread-safe. I don't think this particular approach
> >> to thread safety is useful.
> > 
> > Could you expand on this please?
> 
> readdir vs readdir_r is the canonical example where things went really,
> really wrong.

My opinion is that this is simply historical baggage and POSIX calls this out:
https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xsh_chap02.html#tag_22_02_09_09
~~~
Early proposals had provided "_r" versions for functions that returned pointers
to variable-size buffers without providing a means for determining the required
buffer size. This would have made using such functions exceedingly clumsy,
potentially requiring iteratively calling them with increasingly larger guesses
for the amount of storage required. Hence, sysconf() variables have been
provided for such functions that return the maximum required buffer size.
~~~
This still isn't a great design. They call out later in that text that you
should explicitly provide size information.

So POSIX added readdir_r, got the design wrong, and then we deprecated it.

Today readdir has internal locking, but the returned memory still comes from
the DIR's buffer, so there is still synchronization issues accessing the
returned data.

You could say "memory synchronization" covers this:
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_12
~~~
Applications shall ensure that access to any memory location by more than one
thread of control (threads or processes) is restricted such that no thread of
control can read or modify a memory location while another thread of control
may be modifying it. Such access is restricted using functions that synchronize
thread execution and also synchronize memory with respect to other threads.
~~~
In this case we know by the definition of the interface for readdir that we
can't have multiple threads use the results of readdir without creating a data
race.

See also:
"A.4.12 Memory Synchronization"
https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xbd_chap04.html


The glibc manual doesn't say "MT-safe race:dirstream" because there is library
synchronization provided. We do this even though the results returned to the
other thread _may_ be inconsistent at this point, but that's a memory
synchronization issue with the result of the function.

In summary
- readdir is MT-safe because POSIX says it shoudl be and we use locking.
- the results of readdir require memory synchronization to access otherwise you
get a data race
- readdir_r is unsafe for many other historical reasons and so is deprecated.

We should probably continue this discussion on libc-alpha.

-- 
You are receiving this mail because:
You are on the CC list for the bug.