Re: [PATCH] readdir, readdir64 are thread-safe

[Zack Weinberg <zackw at panix dot com>]

On Tue, Apr 11, 2017 at 9:22 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 04/11/2017 03:16 PM, Zack Weinberg wrote:
>> On Tue, Apr 11, 2017 at 7:36 AM, Florian Weimer <fweimer@redhat.com>
>> wrote:
>>>
>>> They only modify the state in the dirstream argument, and we
>>> generally do not treat this as a reason to mark a function as
>>> not thread-safe.  For an example, see random_r, which is marked
>>> as thread-safe even though the random state is not protected
>>> by a lock.
>>
>> Hmm.  There's two issues here: first, POSIX specifically allows
>> readdir to be not thread-safe (although it's unclear to me what that
>> actually means) so it might be appropriate to keep the annotation
>> around to warn people that there is a portability concern;
>
> The cost of that is that people use readdir_r instead, which is not what we
> want at all.
>
>> second, if
>> you share a DIR object among threads, a call to readdir in one thread
>> will clobber the previous return value, which might still be live in
>> another thread.
>
> That's true for file positions and FILE * objects, too.

Both good points.

>> Is that sufficient reason to call the *function*
>> thread-unsafe?  We don't have any good place to warn people about that
>> *other* than the documentation for readdir.  (Note that the text of
>> the @deftypefun does a very bad job of explaining what the problem
>> is.)
>
> I don't understand your comment about @deftypefun.

Sorry, I meant to complain about this paragraph
(https://sourceware.org/git/?p=glibc.git;a=blob;f=manual/filesys.texi;h=edc7c64d22169bfb9f104321b141a6b4925ce740;hb=38efe8c5a5c1676f54481caa81980a93f7223479#l509):

# In POSIX.1-2008, @code{readdir} is not thread-safe.  In @theglibc{}
# implementation, it is safe to call @code{readdir} concurrently on
# different @var{dirstream}s, but multiple threads accessing the same
# @var{dirstream} result in undefined behavior.  @code{readdir_r} is a
# fully thread-safe alternative, but suffers from poor portability (see
# below).  It is recommended that you use @code{readdir}, with external
# locking if multiple threads access the same @var{dirstream}.

which should really be something like

# @strong{Caution:} The pointer returned by @code{readdir} points to
# a buffer within the @code{DIR} object.  The data in that buffer will
# be overwritten by the next call to @code{readdir}.  You must take care,
# for instance, to copy the @code{d_name} string if you need it later.
#
# Because of this, it is not safe to share a @code{DIR} object among
# multiple threads, unless you use your own locking to ensure that
# no thread calls @code{readdir} while another thread is still using the
# data from the previous call.  In @theglibc{}, it is safe to call
# @code{readdir} from multiple threads as long as each thread uses
# its own @code{DIR} object.  POSIX.1-2008 does not require this to
# be safe, but we are not aware of any operating systems where it
# does not work.
#
# @code{readdir_r} allows you to provide your own buffer for the
# @code{struct dirent}, but it is less portable than @code{readdir}, and
# has problems with very long filenames (see below).  We recommend
# you use @code{readdir}, but do not share @code{DIR} objects.

with matching changes made to the readdir_r documentation.  If we
clarified the text along those lines I'd be fine with calling readdir
thread-safe in the annotations.

zw