This is the mail archive of the
mailing list for the glibc project.
Re: [PATCH] Make pthread_getspecific async-signal-safe
- From: Torvald Riegel <triegel at redhat dot com>
- To: Roland McGrath <roland at hack dot frob dot com>
- Cc: Andrew Hunter <ahh at google dot com>, GNU C Library <libc-alpha at sourceware dot org>, Paul Pluzhnikov <ppluzhnikov at google dot com>
- Date: Tue, 16 Dec 2014 22:05:26 +0100
- Subject: Re: [PATCH] Make pthread_getspecific async-signal-safe
- Authentication-results: sourceware.org; auth=none
- References: <1418756227-17380-1-git-send-email-ahh at google dot com> <CADroS=4w2E5u5iCmoANFy=QeHMy8wP2VxfCh9QustDTdisw2Jw at mail dot gmail dot com> <20141216203128 dot D5E0E2C2448 at topped-with-meat dot com>
On Tue, 2014-12-16 at 12:31 -0800, Roland McGrath wrote:
> I think we might be open to declaring pthread_getspecific async-signal-safe
> as a GNU extension. If we do this, we need to make the manual say very
> explicitly that we're offering this guarantee as part of the glibc API even
> though it is not guaranteed by POSIX. So please include such wording
> changes to manual/threads.texi in your patch.
> The code change itself needs thorough comments explaining what constraints
> the code is (newly) under and exactly what, why, and how it is doing to
> meet them.
> The test program has some common style errors: missing descriptive comment
> in top line; two spaces after a period in comments; space before paren in
> function calls; space after cast.
> It's also generally bad form to write code so important side effects are in
> assert, even though it's not a semantic error in the context of libc tests
> since we'll never build them with -DNDEBUG.
> The test could use some comments about the logic of what it's testing.
Also, you are introducing concurrency, and we require new glibc code to
be data-race-free as defined by C11:
Therefore, please use atomic operations where necessary, and document
how and why that works.
I'm aware that this will be a little challenging given that we don't
have existing support for sigatomic_t in glibc, and that the C and C++
standards could specify what signal handlers are allowed to do more
However, we do need to request the compiler to not reorder memory
accesses (ISTM you are relying on an ordering of accesses to the seq and
data fields in this patch). The simple thing would be to use normal
atomics with acquire/release memory order, but we don't want this
overhead for TLS. So we will need something else. I'd suggest
exploring two options: (1) relaxed memory order atomics and explicit
compiler barriers or (2) atomic operations for sigatomic_t with
something like acq/rel barriers. The latter might be cleaner and more
intuitive to use, but would need more invention.