git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matheus Tavares <matheus.bernardino@usp.br>
To: git@vger.kernel.org
Cc: sandals@crustytoothpaste.net, j6t@kdbg.org,
	jonathantanmy@google.com, peff@peff.net,
	Johannes.Schindelin@gmx.de
Subject: [PATCH 0/2] Make oid_to_hex() thread-safe
Date: Thu, 25 Jun 2020 17:32:55 -0300	[thread overview]
Message-ID: <cover.1593115455.git.matheus.bernardino@usp.br> (raw)
In-Reply-To: <20200625013851.GA9782@camp.crustytoothpaste.net>

Some thread-unsafe functions of our codebase appear very down in the
call stack, which can be hard to notice (or avoid). Thus they are
sometimes used in threaded code unsafely. In this series we add
pthread_once() to compat/win32/ and use it in conjunction with
pthread_key to make a subset of the said functions thread-safe.

As a next step, I would love to make [warning|error|die]_errno()
thread-safe as well. strerror() is not safe on *nix, and there are some
thread functions today that call these (although the actual risk of a
race condition must be very small...)

My idea was to implement a xstrerror() wrapper which calls the
appropriate thread-safe function (dependant on the OS), or even call
strerror() itself but holding a lock to copy the result for a local
buffer (which should be OK as we don't expect contention in strerror).
We could also set a thread local buffer array, as in the second patch of
this series, to excuse callers from allocating/freeing memory.

One concern with this idea is the risk of an infinite recursion if
xstrerror() or any of its childs call [warning|error|die]_errno().
However, if we are only using strerror() and pthread_*() within the
wrapper, there should be no problem, right? Has anyone thought of
other problems with this approach?

Finally, should such change also come with a coccinelle patch to replace
usages of strerror() with xstrerror()? Or better not, as the change
would be too big?

Matheus Tavares (2):
  compat/win32/pthread: add pthread_once()
  hex: make hash_to_hex_algop() and friends thread-safe

 compat/win32/pthread.c | 22 +++++++++++++++++++++
 compat/win32/pthread.h |  5 +++++
 hex.c                  | 45 ++++++++++++++++++++++++++++++++++++++----
 thread-utils.c         | 11 +++++++++++
 thread-utils.h         |  6 ++++++
 5 files changed, 85 insertions(+), 4 deletions(-)

-- 
2.26.2


  reply	other threads:[~2020-06-25 20:33 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-24 22:29 [RFC] Thread safety in some low-level functions Matheus Tavares Bernardino
2020-06-24 22:52 ` Matheus Tavares Bernardino
2020-06-25  1:38   ` brian m. carlson
2020-06-25 20:32     ` Matheus Tavares [this message]
2020-06-25 20:32       ` [PATCH 1/2] compat/win32/pthread: add pthread_once() Matheus Tavares
2020-06-26  5:45         ` Christian Couder
2020-06-26 14:13           ` Matheus Tavares Bernardino
2020-06-25 20:32       ` [PATCH 2/2] hex: make hash_to_hex_algop() and friends thread-safe Matheus Tavares
2020-06-25 23:07         ` brian m. carlson
2020-06-25 23:54           ` Matheus Tavares Bernardino
2020-06-26  0:00             ` Matheus Tavares Bernardino
2020-06-26  6:02         ` Christian Couder
2020-06-26  8:22       ` [PATCH 0/2] Make oid_to_hex() thread-safe Christian Couder
2020-06-26 16:22         ` Matheus Tavares Bernardino
2020-06-26 21:54       ` [PATCH v2 " Matheus Tavares
2020-06-26 21:54         ` [PATCH v2 1/2] compat/win32/pthread: add pthread_once() Matheus Tavares
2020-06-26 21:54         ` [PATCH v2 2/2] hex: make hash_to_hex_algop() and friends thread-safe Matheus Tavares
2020-06-29 15:11           ` Johannes Schindelin
2020-06-30 20:37             ` Matheus Tavares Bernardino
2020-07-01 11:32               ` Johannes Schindelin
2020-07-16 11:29           ` Johannes Schindelin
2020-07-18  3:09             ` Matheus Tavares Bernardino
2020-08-10 14:15               ` Johannes Schindelin
2020-07-18  3:52             ` Matheus Tavares
2020-07-26 17:41               ` René Scharfe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1593115455.git.matheus.bernardino@usp.br \
    --to=matheus.bernardino@usp.br \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=jonathantanmy@google.com \
    --cc=peff@peff.net \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).