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]

[RFC PATCH glibc] pthread_setspecific: Provide signal-safety across keys


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.

So I wonder how acceptable it would be to make just the memory
allocation part of pthread_setspecific signal-safe ? Technically, this
is not required very often, so it should not cause a significant
overhead.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: "Carlos O'Donell" <carlos@redhat.com>
CC: "Ben Maurer" <bmaurer@fb.com>
CC: libc-alpha@sourceware.org
---
 nptl/pthread_setspecific.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/nptl/pthread_setspecific.c b/nptl/pthread_setspecific.c
index 214af3b661..aa5a1f17c8 100644
--- a/nptl/pthread_setspecific.c
+++ b/nptl/pthread_setspecific.c
@@ -61,18 +61,33 @@ __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)
+	  if (level2 == NULL) {
+            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


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