This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[RFC PATCH glibc v2] pthread_setspecific: Provide signal-safety across keys
- From: Mathieu Desnoyers <mathieu dot desnoyers at efficios dot com>
- To: "Carlos O'Donell" <carlos at redhat dot com>, Florian Weimer <fw at deneb dot enyo dot de>
- Cc: Mathieu Desnoyers <mathieu dot desnoyers at efficios dot com>, "Ben Maurer" <bmaurer at fb dot com>, libc-alpha at sourceware dot org
- Date: Tue, 17 Oct 2017 19:24:40 -0400
- Subject: [RFC PATCH glibc v2] pthread_setspecific: Provide signal-safety across keys
- Authentication-results: sourceware.org; auth=none
The intent here is to provide signal-safety against callers to
pthread_{get/set}specific that work on different pthread keys,
without hurting performance of the normal use-cases that do not
care about signal-safety.
One thing to keep in mind is that callers of pthread_{get/set}specific
on a given key can disable signals if they require signal-safety
against operations on their key.
The real problem in my situation (lttng-ust tracer) is having
pthread_setspecific (invoked by the application) do memory allocation
and update pointers without disabling signals when it needs to allocate
"level2". This only happens the first time a key >=
PTHREAD_KEY_2NDLEVEL_SIZE is encountered by a thread. If lttng-ust
tracing inserted into a signal handler nests over this allocation, the
application pthread key specific may be lost, and memory leaked.
Also, use directly mmap rather than calloc to ensure signal-safety.
Make just the memory allocation part of pthread_setspecific signal-safe.
Given this is not required very often, it should not cause a significant
overhead.
Document those new non-POSIX async-signal-safety guarantees in
pthread_getspecific and pthread_setspecific user documentation.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: "Carlos O'Donell" <carlos@redhat.com>
CC: Florian Weimer <fw@deneb.enyo.de>
CC: "Ben Maurer" <bmaurer@fb.com>
CC: libc-alpha@sourceware.org
---
manual/threads.texi | 18 ++++++++++--------
nptl/pthread_create.c | 3 ++-
nptl/pthread_setspecific.c | 24 +++++++++++++++++++++---
3 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/manual/threads.texi b/manual/threads.texi
index 769d974d50..62504ecfa9 100644
--- a/manual/threads.texi
+++ b/manual/threads.texi
@@ -53,20 +53,22 @@ is it called during thread exit.
@c pthread_getspecific ok
Return the thread-specific data associated with @var{key} in the calling
thread.
+As a non-POSIX extension, @code{pthread_getspecific} is async-signal safe.
@end deftypefun
@deftypefun int pthread_setspecific (pthread_key_t @var{key}, const void *@var{value})
@standards{POSIX, pthread.h}
-@safety{@prelim{}@mtsafe{}@asunsafe{@asucorrupt{} @ascuheap{}}@acunsafe{@acucorrupt{} @acsmem{}}}
-@c pthread_setspecific @asucorrupt @ascuheap @acucorrupt @acsmem
-@c a level2 block may be allocated by a signal handler after
-@c another call already made a decision to allocate it, thus losing
-@c the allocated value. the seq number is updated before the
-@c value, which might cause an earlier-generation value to seem
-@c current if setspecific is cancelled or interrupted by a signal
+@safety{@prelim{}@mtsafe{}@asunsafe{@asucorrupt{}}@acunsafe{@acucorrupt{} @acsmem{}}}
+@c pthread_setspecific @asucorrupt @acucorrupt @acsmem
+@c the seq number is updated before the value, which might cause
+@c an earlier-generation value to seem current if setspecific is
+@c cancelled or interrupted by a signal
@c KEY_UNUSED ok
-@c calloc dup @ascuheap @acsmem
+@c dup @acsmem
Associate the thread-specific @var{value} with @var{key} in the calling thread.
+As a non-POSIX extension, @code{pthread_setspecific} is async-signal safe for
+concurrent invocations against different @var{key}, but not async-signal
+safe for concurrent invocations against the same @var{key}.
@end deftypefun
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 992331e280..78abc07f91 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -33,6 +33,7 @@
#include <default-sched.h>
#include <futex-internal.h>
#include "libioP.h"
+#include <sys/mman.h>
#include <shlib-compat.h>
@@ -327,7 +328,7 @@ __nptl_deallocate_tsd (void)
{
/* The first block is allocated as part of the thread
descriptor. */
- free (level2);
+ (void) munmap (level2, PTHREAD_KEY_2NDLEVEL_SIZE * sizeof (*level2));
THREAD_SETMEM_NC (self, specific, cnt, NULL);
}
}
diff --git a/nptl/pthread_setspecific.c b/nptl/pthread_setspecific.c
index 214af3b661..2beb7f97fc 100644
--- a/nptl/pthread_setspecific.c
+++ b/nptl/pthread_setspecific.c
@@ -18,6 +18,7 @@
#include <errno.h>
#include <stdlib.h>
+#include <sys/mman.h>
#include "pthreadP.h"
@@ -61,18 +62,35 @@ __pthread_setspecific (pthread_key_t key, const void *value)
level2 = THREAD_GETMEM_NC (self, specific, idx1st);
if (level2 == NULL)
{
+ sigset_t ss, old_ss;
+
if (value == NULL)
/* We don't have to do anything. The value would in any case
be NULL. We can save the memory allocation. */
return 0;
+ /* Ensure that pthread_setspecific and pthread_getspecific are
+ signal-safe when used on different keys. */
+ sigfillset (&ss);
+ pthread_sigmask (SIG_BLOCK, &ss, &old_ss);
+ /* Read level2 again with signals disabled. */
+ level2 = THREAD_GETMEM_NC (self, specific, idx1st);
+ if (level2 != NULL)
+ goto skip_resize;
+
level2
- = (struct pthread_key_data *) calloc (PTHREAD_KEY_2NDLEVEL_SIZE,
- sizeof (*level2));
- if (level2 == NULL)
+ = (struct pthread_key_data *) mmap (NULL,
+ PTHREAD_KEY_2NDLEVEL_SIZE * sizeof (*level2),
+ PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE,
+ -1, 0);
+ if (level2 == MAP_FAILED) {
+ pthread_sigmask (SIG_SETMASK, &old_ss, NULL);
return ENOMEM;
+ }
THREAD_SETMEM_NC (self, specific, idx1st, level2);
+skip_resize:
+ pthread_sigmask (SIG_SETMASK, &old_ss, NULL);
}
/* Pointer to the right array element. */
--
2.11.0