From patchwork Thu Jun 25 20:32:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matheus Tavares X-Patchwork-Id: 11626091 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 07D09161F for ; Thu, 25 Jun 2020 20:33:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E54EB20791 for ; Thu, 25 Jun 2020 20:33:11 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=usp-br.20150623.gappssmtp.com header.i=@usp-br.20150623.gappssmtp.com header.b="GANYRG2d" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2407143AbgFYUdK (ORCPT ); Thu, 25 Jun 2020 16:33:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46792 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2407140AbgFYUdK (ORCPT ); Thu, 25 Jun 2020 16:33:10 -0400 Received: from mail-qt1-x841.google.com (mail-qt1-x841.google.com [IPv6:2607:f8b0:4864:20::841]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3D1C6C08C5C1 for ; Thu, 25 Jun 2020 13:33:10 -0700 (PDT) Received: by mail-qt1-x841.google.com with SMTP id i3so5762275qtq.13 for ; Thu, 25 Jun 2020 13:33:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=usp-br.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=bnc+t4YwONAiHUY23n0I/z0J1VF6mvI+wvTSBvo23Bk=; b=GANYRG2dEVNMTAzTO7zKysntGMzc6B4oHEPEkdxEOMua5EzXnfhJy7NUI7jYF0UAyz OFDPdxJLI4O370zuSecVgQuSYUtdEQA0Gq+Anvj+fTuAUaoh9z0BTOKF39vpxQ4DBtt6 ln+5ei8FEWtK8l74JrHYUnx74FrS+81+UW3Yt4GRWfYzEDGFzKSiqwgQokPcgBk9bfkG 0XQ+QFz/QEMZSsyeDCh4b0kDDPEGGYLyjahEj3JpLmzSSaqfcsU7fKkfpBKPghAA9FEX nYAF1lllOUsF5h5FkNQo1FzGjpy3UKHZyiWRBI4Y4Mj3YUkXv2pzoqibv6e8mWGPpRk6 U4/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=bnc+t4YwONAiHUY23n0I/z0J1VF6mvI+wvTSBvo23Bk=; b=QbJ/3SrsUWuoIYdxtG9UCl0PDcKL9D+gT8lTyapAI5bchL+0XgFqDEVH4g505erVvr 69HVCOJCcCGBD8jZujXyecTapP7SuL/wKPBrwJ7tR4CVjM0KJxc0okF295TKedlqV9ZN U3VwrW9X7e3lNKlm4svbYgf1PwJRSnStp6lmfAGleoX+AtkofvgKJCjyS9dONBS6gZe9 xA+NNMzRvZWp4cikAHkqXC++3M85DRyYvBBHnWzn1hL9V3z5Y7+SHRzP0RD0lw21MR6r K/jaIX9Q2D5RfPapGP9SHWc+8BraX3l9eKCkVeFOTCqQJbXThJOtd8c57GEFQ2Sj/h7V B30A== X-Gm-Message-State: AOAM532D+0b2v6+svtAjOZPSg5r+UNFNwGft+Tcizu29Eycb06vN79Nc FfNESRuvAVRHzhoQdtGw9hhYdhxtxQ8= X-Google-Smtp-Source: ABdhPJxdrOjkY3NbINpHeG13WS9Y+hhJbXcFLHILB+ZxhaFgEYcYKo5J3OeeQi+ko1FDkoinCddIiA== X-Received: by 2002:ac8:2aa9:: with SMTP id b38mr15724067qta.49.1593117188995; Thu, 25 Jun 2020 13:33:08 -0700 (PDT) Received: from mango.spo.virtua.com.br ([2804:14c:81:9a16::4]) by smtp.gmail.com with ESMTPSA id n63sm6745118qkn.104.2020.06.25.13.33.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Jun 2020 13:33:08 -0700 (PDT) From: Matheus Tavares To: git@vger.kernel.org Cc: sandals@crustytoothpaste.net, j6t@kdbg.org, jonathantanmy@google.com, peff@peff.net, Johannes.Schindelin@gmx.de Subject: [PATCH 1/2] compat/win32/pthread: add pthread_once() Date: Thu, 25 Jun 2020 17:32:56 -0300 Message-Id: X-Mailer: git-send-email 2.26.2 In-Reply-To: References: MIME-Version: 1.0 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Signed-off-by: Matheus Tavares --- Note: the pthread_once() function is adapted from: https://git.libav.org/?p=libav.git;a=commitdiff;h=b22693b06d1e5d73454a65c203b4d31c1ca5b69a Which is LGPLv2.1. Should I add any notice/acknowledgment somewhere, besides the comment I added above the function? compat/win32/pthread.c | 22 ++++++++++++++++++++++ compat/win32/pthread.h | 5 +++++ thread-utils.c | 11 +++++++++++ thread-utils.h | 6 ++++++ 4 files changed, 44 insertions(+) diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c index 2e7eead42c..5a7ecbd999 100644 --- a/compat/win32/pthread.c +++ b/compat/win32/pthread.c @@ -56,3 +56,25 @@ pthread_t pthread_self(void) t.tid = GetCurrentThreadId(); return t; } + +/* Adapted from libav's compat/w32pthreads.h. */ +int pthread_once(pthread_once_t *once_control, void (*init_routine)(void)) +{ + BOOL pending = FALSE; + int ret = 0; + + if(!InitOnceBeginInitialize(once_control, 0, &pending, NULL)) { + ret = err_win_to_posix(GetLastError()); + goto out; + } + + if (pending) + init_routine(); + + if(!InitOnceComplete(once_control, 0, NULL)) + ret = err_win_to_posix(GetLastError()); + +out: + /* POSIX doesn't allow pthread_once() to return EINTR */ + return ret == EINTR ? EIO : ret; +} diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h index 737983d00b..c50f1e89c7 100644 --- a/compat/win32/pthread.h +++ b/compat/win32/pthread.h @@ -40,6 +40,11 @@ typedef int pthread_mutexattr_t; #define pthread_cond_signal WakeConditionVariable #define pthread_cond_broadcast WakeAllConditionVariable +#define pthread_once_t INIT_ONCE + +#define PTHREAD_ONCE_INIT INIT_ONCE_STATIC_INIT +int pthread_once(pthread_once_t *once_control, void (*init_routine)(void)); + /* * Simple thread creation implementation using pthread API */ diff --git a/thread-utils.c b/thread-utils.c index 5329845691..937deb3f2e 100644 --- a/thread-utils.c +++ b/thread-utils.c @@ -122,4 +122,15 @@ int dummy_pthread_join(pthread_t pthread, void **retval) return ENOSYS; } +int nothreads_pthread_once(pthread_once_t *once_control, + void (*init_routine)(void)) +{ + if (*once_control == 1) + return 0; + + init_routine(); + *once_control = 1; + return 0; +} + #endif diff --git a/thread-utils.h b/thread-utils.h index 4961487ed9..bab9dc5e4d 100644 --- a/thread-utils.h +++ b/thread-utils.h @@ -19,6 +19,7 @@ #define pthread_mutex_t int #define pthread_cond_t int #define pthread_key_t int +#define pthread_once_t int #define pthread_mutex_init(mutex, attr) dummy_pthread_init(mutex) #define pthread_mutex_lock(mutex) @@ -48,6 +49,11 @@ int dummy_pthread_join(pthread_t pthread, void **retval); int dummy_pthread_init(void *); +#define PTHREAD_ONCE_INIT 0 +int nothreads_pthread_once(pthread_once_t *once_control, + void (*init_routine)(void)); +#define pthread_once(once, routine) nothreads_pthread_once(once, routine) + #endif int online_cpus(void); From patchwork Thu Jun 25 20:32:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matheus Tavares X-Patchwork-Id: 11626095 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 8627F912 for ; Thu, 25 Jun 2020 20:33:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6D53E2072E for ; Thu, 25 Jun 2020 20:33:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=usp-br.20150623.gappssmtp.com header.i=@usp-br.20150623.gappssmtp.com header.b="yr2T8q7k" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2407146AbgFYUdP (ORCPT ); Thu, 25 Jun 2020 16:33:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46804 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2407140AbgFYUdO (ORCPT ); Thu, 25 Jun 2020 16:33:14 -0400 Received: from mail-qk1-x741.google.com (mail-qk1-x741.google.com [IPv6:2607:f8b0:4864:20::741]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BCE10C08C5C1 for ; Thu, 25 Jun 2020 13:33:14 -0700 (PDT) Received: by mail-qk1-x741.google.com with SMTP id 145so4245565qke.9 for ; Thu, 25 Jun 2020 13:33:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=usp-br.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=6p4gqrkOITNw6tnB9xnpq/Qeo91MuyX0Kj2dGHJu5Xg=; b=yr2T8q7kaWnW8E0iEssJWwHi3emsl4rg3LRu+xqb7N2uoXkxml5VehymecEzaXkm10 Yypi+vX9Ifw8Cs+zjIWS5AlvJNiJCWB+8XdDwX7I2iCO/QlOp8oVsgc82E8y/4Qa9klL h8YSzFSfLqvaMgEFoOTcB2KXKHdems1Oz4QjRRe4PiOi9Sc+AJWs3aEaeB8NRCpe/kP1 /hzOU+N029IWT/Ax51JAVJrzqkzUIZfEfwcq7Y7UWbl/HgF1mvbsrZKQL5+M6J7jGO7r GDTs5a8whHyAPKA92KmPWVhY0i1iAMb2/znhvlFVmb31m2XeQUs8fjArecV75AI9BN4w jlsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=6p4gqrkOITNw6tnB9xnpq/Qeo91MuyX0Kj2dGHJu5Xg=; b=kIIhnuVQPd7+HqWL3cTtxMq3CcADVcQ+N5Y0T9EQ1Ov0MI5pwFy/tJVXmhC77FMZ9M +KzgVlGp+II9AB36u1tVT6FmYywz5aToCXqvSqi+sj5lLdPGx7kNRzbLw5HXOzj23iAY 0kWI5hoj465WwiR+EHvDO8uXoxm7NERhpiMJq1Pxp6u372qpzwJ5nT9/975Axat2eYxl 2uYDUELxpii0/i7OiS/B479UsZMk8V/wuyjU0hSv07iAmzVl+3KhZOZZbstaIBGShTiU YJGou62qWB1RRC9nLObI00MzBN5BZu1U0mm8u5u9Q8cTaI1X/vFwCtKHPB/3dcYJZ/Q5 zd4w== X-Gm-Message-State: AOAM531/CmzYWp0GcGsBJMUqz7u/lBmTgvdSGUWxBztoyNDFkIG4AiO6 CUHQmZ+P9i61IVwgbaw9YKLIwcALUxg= X-Google-Smtp-Source: ABdhPJzGbTAOgOqxDTzvyYCAz+EsB0uxSwyEzy8Hh1eNbTKJ5e/F9awKVy+cACEz1/eLZ3d8BzUNJg== X-Received: by 2002:a37:474d:: with SMTP id u74mr15573056qka.195.1593117193478; Thu, 25 Jun 2020 13:33:13 -0700 (PDT) Received: from mango.spo.virtua.com.br ([2804:14c:81:9a16::4]) by smtp.gmail.com with ESMTPSA id n63sm6745118qkn.104.2020.06.25.13.33.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Jun 2020 13:33:12 -0700 (PDT) From: Matheus Tavares To: git@vger.kernel.org Cc: sandals@crustytoothpaste.net, j6t@kdbg.org, jonathantanmy@google.com, peff@peff.net, Johannes.Schindelin@gmx.de, Fredrik Kuivinen Subject: [PATCH 2/2] hex: make hash_to_hex_algop() and friends thread-safe Date: Thu, 25 Jun 2020 17:32:57 -0300 Message-Id: <0104cd9c763aee220a2df357834c79b10695ee35.1593115455.git.matheus.bernardino@usp.br> X-Mailer: git-send-email 2.26.2 In-Reply-To: References: MIME-Version: 1.0 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org hash_to_hex_algop() returns a static buffer, relieving callers from the responsibility of freeing memory after use. But the current implementation uses the same static data for all threads and, thus, is not thread-safe. We could avoid using this function and its wrappers in threaded code, but they are sometimes too deep in the call stack to be noticed or even avoided. For example, we can take a look at the number of oid_to_hex() calls, which calls hash_to_hex_algop(): $ git grep 'oid_to_hex(' | wc -l 818 Although these functions don't seem to be causing problems out there for now (at least not reported), making them thread-safe makes the codebase more robust against race conditions. We can easily do that replicating the static buffer in each thread's local storage. Original-patch-by: Fredrik Kuivinen Signed-off-by: Matheus Tavares --- hex.c | 45 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/hex.c b/hex.c index da51e64929..1094ed25bd 100644 --- a/hex.c +++ b/hex.c @@ -136,12 +136,49 @@ char *oid_to_hex_r(char *buffer, const struct object_id *oid) return hash_to_hex_algop_r(buffer, oid->hash, the_hash_algo); } +struct hexbuf_array { + int idx; + char bufs[4][GIT_MAX_HEXSZ + 1]; +}; + +#ifdef HAVE_THREADS +static pthread_key_t hexbuf_array_key; +static pthread_once_t hexbuf_array_once = PTHREAD_ONCE_INIT; + +void init_hexbuf_array_key(void) +{ + if (pthread_key_create(&hexbuf_array_key, free)) + die(_("failed to initialize threads' key for hash to hex conversion")); +} + +#else +static struct hexbuf_array default_hexbuf_array; +#endif + char *hash_to_hex_algop(const unsigned char *hash, const struct git_hash_algo *algop) { - static int bufno; - static char hexbuffer[4][GIT_MAX_HEXSZ + 1]; - bufno = (bufno + 1) % ARRAY_SIZE(hexbuffer); - return hash_to_hex_algop_r(hexbuffer[bufno], hash, algop); + struct hexbuf_array *ha; + +#ifdef HAVE_THREADS + void *value; + + if (pthread_once(&hexbuf_array_once, init_hexbuf_array_key)) + die(_("failed to initialize threads' key for hash to hex conversion")); + + value = pthread_key_getspecific(&hexbuf_array_key); + if (value) { + ha = (struct hexbuf_array *) value; + } else { + ha = xmalloc(sizeof(*ha)); + if (pthread_key_setspecific(&hexbuf_array_key, (void *)ha)) + die(_("failed to set thread buffer for hash to hex conversion")); + } +#else + ha = &default_hexbuf_array; +#endif + + ha->idx = (ha->idx + 1) % ARRAY_SIZE(ha->bufs); + return hash_to_hex_algop_r(ha->bufs[ha->idx], hash, algop); } char *hash_to_hex(const unsigned char *hash)