From patchwork Tue Oct 17 21:29:19 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathieu Desnoyers X-Patchwork-Id: 23652 Received: (qmail 91909 invoked by alias); 17 Oct 2017 21:31:01 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 91853 invoked by uid 89); 17 Oct 2017 21:31:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mail.efficios.com From: Mathieu Desnoyers To: "Carlos O'Donell" Cc: Mathieu Desnoyers , "Ben Maurer" , libc-alpha@sourceware.org Subject: [RFC PATCH glibc] pthread_setspecific: Provide signal-safety across keys Date: Tue, 17 Oct 2017 17:29:19 -0400 Message-Id: <20171017212919.741-1-mathieu.desnoyers@efficios.com> 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 CC: "Carlos O'Donell" CC: "Ben Maurer" 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. */