don't use libc.threads_minus_1 as relaxed atomic for skipping locks
authorRich Felker <dalias@aerifal.cx>
Fri, 22 May 2020 03:32:45 +0000 (23:32 -0400)
committerRich Felker <dalias@aerifal.cx>
Fri, 22 May 2020 21:39:57 +0000 (17:39 -0400)
commite01b5939b38aea5ecbe41670643199825874b26c
tree100e5d227741abd1653158b2e236ba3c24621113
parent4d5aa20a94a2d3fae3e69289dc23ecafbd0c16c4
don't use libc.threads_minus_1 as relaxed atomic for skipping locks

after all but the last thread exits, the next thread to observe
libc.threads_minus_1==0 and conclude that it can skip locking fails to
synchronize with any changes to memory that were made by the
last-exiting thread. this can produce data races.

on some archs, at least x86, memory synchronization is unlikely to be
a problem; however, with the inline locks in malloc, skipping the lock
also eliminated the compiler barrier, and caused code that needed to
re-check chunk in-use bits after obtaining the lock to reuse a stale
value, possibly from before the process became single-threaded. this
in turn produced corruption of the heap state.

some uses of libc.threads_minus_1 remain, especially for allocation of
new TLS in the dynamic linker; otherwise, it could be removed
entirely. it's made non-volatile to reflect that the remaining
accesses are only made under lock on the thread list.

instead of libc.threads_minus_1, libc.threaded is now used for
skipping locks. the difference is that libc.threaded is permanently
true once an additional thread has been created. this will produce
some performance regression in processes that are mostly
single-threaded but occasionally creating threads. in the future it
may be possible to bring back the full lock-skipping, but more care
needs to be taken to produce a safe design.
src/internal/libc.h
src/malloc/malloc.c
src/thread/__lock.c