This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Make pthread_getspecific async-signal-safe


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.

I agree.

Also, you are introducing concurrency, and we require new glibc code to
be data-race-free as defined by C11:
https://sourceware.org/glibc/wiki/Concurrency

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
clearly.  

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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]