Re: thread safety level of fwide

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

On 11/19/2014 03:58 PM, Carlos O'Donell wrote:
> On 11/19/2014 03:32 PM, Alexandre Oliva wrote:
>> On Nov 19, 2014, MaShimiao <mashimiao.fnst@cn.fujitsu.com> wrote:
>>
>>> After reading it's source code, I think it should be marked with race:stream.
>>> The reasoning is fwide() uses fp several times inside without holding the lock over all of them.
>>
>>> How do you think about that?
>>
>> The uses of fp, although technically racy, are well-defined, because of
>> the way stream orientation works: once it's decided, it never changes,
>> so it is safe to test, before taking a lock, whether it is already
>> decided and use the settled value if so.  It is only if it is not
>> decided yet (when _mode is zero) and we want to set a value (when mode
>> is nonzero) that we have to resort to mutual exclusion to serialize mode
>> settings.
>>
>> Torvald, this is another of those âtechnically a race, but arguably
>> safeâ cases that you've been looking for in glibc.  This idiom of using
>> âundecidedâ initial values tested before taking locks is quite common in
>> IOstreams, in TLS infrastructure, and in various __have_*
>> feature-testing caches throughout glibc.
> 
> Do we ignore the race against fclose?

In part to answer my own question, the documented behaviour for fclose
is that any concurrent access, given lack of happens-before or happens-after
could result in the use of a FILE* that was already closed and thereby
invoking undefined behaviour.

I guess therefore that fclose should not be considered important for this
dicussion, and that the above is racy.

I say it is racy because I firmly believe glibc should be annotating with
atomic accesses all of those fields it expects to read atomically, and that
we should attempt to be data-race free internally by using such annotations.
There could have been a write to fp->_mode by another thread and that creates
a data race, despite the fact that it work (similar to the way we lazy update
PLT slots without atomic instructions). It's just wrong and should be annotated
even if such annotation does nothing initially.

Cheers,
Carlos.