Jeff King wrote:
> I don't think it makes sense for git-clone to do this itself. If we are
> going to say "SIGPIPE should default to SIG_DFL on startup" then we
> should do it as the very first thing in the git wrapper, not just for
> git-clone. That gives each git program a known starting point of
> behavior.
Here's what that might look like.
-- 8< --
Subject: SIGPIPE and other fatal signals should default to SIG_DFL
The intuitive behavior when a git command receives a fatal
signal is for it to die.
Indeed, that is the default handling. However, POSIX semantics
allow the parent of a process to override that default by
ignoring a signal, since ignored signals are preserved by fork() and
exec(). For example, Python 2.6 and 2.7's os.popen() runs a shell
with SIGPIPE ignored (Python issue 1736483).
Worst of all, in such a situation, many git commands use
sigchain_push_common() to run cleanup actions (removing temporary
files, discarding locks, that kind of thing) before passing control to
the original handler; if that handler ignores the signal rather than
exiting, then execution will continue in an unplanned-for and unusual
state.
So give the signals in question default handling at the start of
main(), before sigchain_push_common can be called.
NEEDSWORK:
- imposes an obnoxious maintenance burden. New non-builtins
that might call sigchain_push_common() would need to have
default handling restored at the start of main().
- needs tests, especially for interaction with "git daemon"
client shutdown
Suggested-by: Jeff King <peff@xxxxxxxx>
Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
---
Currently git daemon uses SIG_IGN state on SIGTERM to protect
children with active connections. Why isn't that causing the same
sort of problems as os.popen() causes?
check-racy.c | 1 +
daemon.c | 1 +
fast-import.c | 1 +
git.c | 2 ++
http-backend.c | 1 +
http-fetch.c | 1 +
http-push.c | 1 +
imap-send.c | 1 +
remote-curl.c | 1 +
shell.c | 2 ++
show-index.c | 2 ++
upload-pack.c | 1 +
12 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/check-racy.c b/check-racy.c
index 00d92a1..f1ad9d5 100644
--- a/check-racy.c
+++ b/check-racy.c
@@ -6,6 +6,7 @@ int main(int ac, char **av)
int dirty, clean, racy;
dirty = clean = racy = 0;
+ sigchain_push_common(SIG_DFL);
read_cache();
for (i = 0; i < active_nr; i++) {
struct cache_entry *ce = active_cache[i];
diff --git a/daemon.c b/daemon.c
index 7ccd097..7719f33 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1001,6 +1001,7 @@ int main(int argc, char **argv)
gid_t gid = 0;
int i;
+ sigchain_push_common(SIG_DFL);
git_extract_argv0_path(argv[0]);
for (i = 1; i < argc; i++) {
diff --git a/fast-import.c b/fast-import.c
index dc48245..ce28759 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -3027,6 +3027,7 @@ int main(int argc, const char **argv)
{
unsigned int i;
+ sigchain_push_common(SIG_DFL);
git_extract_argv0_path(argv[0]);
if (argc == 2 && !strcmp(argv[1], "-h"))
diff --git a/git.c b/git.c
index e08d0f1..a8677fb 100644
--- a/git.c
+++ b/git.c
@@ -502,6 +502,8 @@ int main(int argc, const char **argv)
if (!cmd)
cmd = "git-help";
+ sigchain_push_common(SIG_DFL);
+
/*
* "git-xxxx" is the same as "git xxxx", but we obviously:
*
diff --git a/http-backend.c b/http-backend.c
index 14c90c2..b03455a 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -550,6 +550,7 @@ int main(int argc, char **argv)
char *cmd_arg = NULL;
int i;
+ sigchain_push_common(SIG_DFL);
git_extract_argv0_path(argv[0]);
set_die_routine(die_webcgi);
diff --git a/http-fetch.c b/http-fetch.c
index 762c750..aca4e8f 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -24,6 +24,7 @@ int main(int argc, const char **argv)
int get_verbosely = 0;
int get_recover = 0;
+ sigchain_push_common(SIG_DFL);
git_extract_argv0_path(argv[0]);
while (arg < argc && argv[arg][0] == '-') {
diff --git a/http-push.c b/http-push.c
index c9bcd11..a9d0894 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1791,6 +1791,7 @@ int main(int argc, char **argv)
struct remote *remote;
char *rewritten_url = NULL;
+ sigchain_push_common(SIG_DFL);
git_extract_argv0_path(argv[0]);
repo = xcalloc(sizeof(*repo), 1);
diff --git a/imap-send.c b/imap-send.c
index 71506a8..eb13adc 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1538,6 +1538,7 @@ int main(int argc, char **argv)
int nongit_ok;
+ sigchain_push_common(SIG_DFL);
git_extract_argv0_path(argv[0]);
if (argc != 1)
usage(imap_send_usage);
diff --git a/remote-curl.c b/remote-curl.c
index 04d4813..804492d 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -787,6 +787,7 @@ int main(int argc, const char **argv)
struct strbuf buf = STRBUF_INIT;
int nongit;
+ sigchain_push_common(SIG_DFL);
git_extract_argv0_path(argv[0]);
setup_git_directory_gently(&nongit);
if (argc < 2) {
diff --git a/shell.c b/shell.c
index dea4cfd..9798b99 100644
--- a/shell.c
+++ b/shell.c
@@ -137,6 +137,8 @@ int main(int argc, char **argv)
int devnull_fd;
int count;
+ sigchain_push_common(SIG_DFL);
+
/*
* Always open file descriptors 0/1/2 to avoid clobbering files
* in die(). It also avoids not messing up when the pipes are
diff --git a/show-index.c b/show-index.c
index 4c0ac13..7d48c88 100644
--- a/show-index.c
+++ b/show-index.c
@@ -11,6 +11,8 @@ int main(int argc, char **argv)
unsigned int version;
static unsigned int top_index[256];
+ sigchain_push_common(SIG_DFL);
+
if (argc != 1)
usage(show_index_usage);
if (fread(top_index, 2 * 4, 1, stdin) != 1)
diff --git a/upload-pack.c b/upload-pack.c
index f05e422..4c764f7 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -682,6 +682,7 @@ int main(int argc, char **argv)
int i;
int strict = 0;
+ sigchain_push_common(SIG_DFL);
git_extract_argv0_path(argv[0]);
read_replace_refs = 0;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html