From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: iwiedler@gitlab.com
Subject: [PATCH 0/1] Async-signal safety in signal handlers
Date: Fri, 7 Jan 2022 11:53:35 +0100 [thread overview]
Message-ID: <cover.1641552500.git.ps@pks.im> (raw)
[-- Attachment #1: Type: text/plain, Size: 2328 bytes --]
Hi,
we have recently observed a Git process which has been hanging around
for more than a month on one of our servers in production. A backtrace
showed that the git-fetch(1) process was deadlocked in its signal
handler while trying to free memory. Functions like malloc, free and
most I/O functions aren't reentrant though, which means they must not be
executed in async signal handlers as specified in signal-safety(7).
The fix for git-fetch(1) is rather simple: we can just unlink(2) the
lockfiles, which is indeed allowed, but skip free'ing memory. But in
fact, this is a wider issue we have: we mostly didn't pay attention to
those restrictions, and thus we freely call non-async-signal-safe
functions. It's less clear what to do about this in most of the cases
though:
- git-clone(1) tries to clean up the ".git" directory and its worktree
on being killed, but needs to allocate memory to compute corresponding
paths. We can try to preallocate the buffer, but it's not clear
whether there is a proper upper boundary.
- git-gc(1) will try to commit "gc.log" and write to stderr, both of
which aren't allowed. I think we'll have to just bail and leave it
behind in a partially-written state.
- git-repack(1) tries to remove "pack/.tmp-*" files, calling opendir(3P),
readdir(3P), closedir(3P) and allocates memory. We probably have to
keep track of all temporary files we create in a global list, which we
can then access in our signal handler.
- git-worktree(1) is doing the same as git-clone(1), trying to prune the
new worktree if it's killed. Again, we'd probably have to preallocate
a buffer to compute paths.
- HTTP pushes do all sorts of HTTP requests in their signal handler to
unlock the remote server. I don't really see what to do about this
except drop the code -- setting a global "please clean up and exit
now" flags is probably not going to fly well.
The tempfiles and tmp-objdir code already handles signals correctly.
Patrick
Patrick Steinhardt (1):
fetch: fix deadlock when cleaning up lockfiles in async signals
builtin/clone.c | 2 +-
builtin/fetch.c | 17 +++++++++++------
transport.c | 11 ++++++++---
transport.h | 14 +++++++++++++-
4 files changed, 33 insertions(+), 11 deletions(-)
--
2.34.1
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next reply other threads:[~2022-01-07 10:54 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-07 10:53 Patrick Steinhardt [this message]
2022-01-07 10:55 ` [PATCH 1/1] fetch: fix deadlock when cleaning up lockfiles in async signals Patrick Steinhardt
2022-01-07 11:14 ` brian m. carlson
2022-01-07 22:41 ` Taylor Blau
2022-01-08 10:54 ` Phillip Wood
2022-01-11 2:11 ` Taylor Blau
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover.1641552500.git.ps@pks.im \
--to=ps@pks.im \
--cc=git@vger.kernel.org \
--cc=iwiedler@gitlab.com \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).