Re: [PATCH 2/2] manual: Document __libc_single_threaded

[Florian Weimer via Libc-alpha <libc-alpha@sourceware.org>]

* Rich Felker:

>> This still has consequences for setxid safety which is why musl now
>> fully synchronizes the existing threads list. But if you're not using
>> the thread count for that, it's not an issue. Indeed I think
>> SYS_membarrier is a solution here, but if it's not supported or
>> blocked by seccomp then __libc_single_threaded must not be made true
>> again at this time.
>
> Uhg, SYS_membarrier is *not* a solution here. The problem is far
> worse, because the user of __libc_single_threaded potentially lacks
> *compiler barriers* too.
>
> Consider something like:
>
> 	if (!__libc_single_threaded) { lock(); need_unlock=1; }
> 	x = *p;
> 	if (need_unlock) unlock();
> 	/* ... */
> 	if (!__libc_single_threaded) { lock(); need_unlock=1; }
> 	x = *p;
> 	if (need_unlock) unlock();
>
> Here, in the case where __libc_single_threaded is true the second time
> around, there is no (memory or compiler) acquire barrier between the
> first access to *p and the second. Thus the compiler can (and actually
> does! I don't have a minimal PoC but musl actually just hit a bug very
> close to this) omit the second load from memory, and uses the cached
> value, which may be incorrect because the exiting thread modified it.
>
> This could potentially be avoided with complex contracts about
> barriers needed to use __libc_single_threaded, but it seems highly
> error-prone.

Well, yes.  It's clearly a data race if the implementation sets
__libc_single_threaded directly from an exiting thread.  I don't see a
way around that.

Our discussion focused on the problem that observing a thread count of 1
in pthread_join does not necessarily mean that it is safe to assume at
this point that the process is single-threaded, in glibc's
implementation that uses a simple __nptl_nthreads counter decremented on
the thread itself.  This does not cause a low-level data race directly,
but is potentially still incorrect (I'm not quite sure yet).

In glibc, we annotate many functions with __attribute__ ((leaf)),
implicitly via __THROW.  None of these functions may reset
__libc_single_threaded.  I expect that many compilers have a built-in
list of standard functions they treat as leaf functions.  This means
that these functions cannot write in practice to __libc_single_threaded
(or any other global variable apart from errno).  Not following this
rule would result in undefined behavior, similar to an actual data race
in the memory model.

A compiler cannot treat pthread_create as a leaf function, so the simple
implementation of __libc_single_threaded I posted should be fine in this
regard.

Thanjs,
Florian