From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 7 Oct 2015 20:38:16 +0000 (-0700)
Subject: Merge branch 'ti/glibc-stdio-mutex-from-signal-handler'
X-Git-Tag: v2.7.0-rc0~127
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/2b72dbbcf3a1d6c6813db2c13afaa1d0d8585f7b?hp=-c

Merge branch 'ti/glibc-stdio-mutex-from-signal-handler'

Allocation related functions and stdio are unsafe things to call
inside a signal handler, and indeed killing the pager can cause
glibc to deadlock waiting on allocation mutex as our signal handler
tries to free() some data structures in wait_for_pager().  Reduce
these unsafe calls.

* ti/glibc-stdio-mutex-from-signal-handler:
pager: don't use unsafe functions in signal handlers
---

2b72dbbcf3a1d6c6813db2c13afaa1d0d8585f7b
diff --combined pager.c
index 27d4c8a17a,0f789c3ed4..e425070528
--- a/pager.c
+++ b/pager.c
@@@ -14,19 -14,29 +14,29 @@@
  static const char *pager_argv[] = { NULL, NULL };
  static struct child_process pager_process = CHILD_PROCESS_INIT;
  
- static void wait_for_pager(void)
+ static void wait_for_pager(int in_signal)
  {
- 	fflush(stdout);
- 	fflush(stderr);
+ 	if (!in_signal) {
+ 		fflush(stdout);
+ 		fflush(stderr);
+ 	}
  	/* signal EOF to pager */
  	close(1);
  	close(2);
- 	finish_command(&pager_process);
+ 	if (in_signal)
+ 		finish_command_in_signal(&pager_process);
+ 	else
+ 		finish_command(&pager_process);
+ }
+ 
+ static void wait_for_pager_atexit(void)
+ {
+ 	wait_for_pager(0);
  }
  
  static void wait_for_pager_signal(int signo)
  {
- 	wait_for_pager();
+ 	wait_for_pager(1);
  	sigchain_pop(signo);
  	raise(signo);
  }
@@@ -90,7 -100,7 +100,7 @@@ void setup_pager(void
  
  	/* this makes sure that the parent terminates after the pager */
  	sigchain_push_common(wait_for_pager_signal);
- 	atexit(wait_for_pager);
+ 	atexit(wait_for_pager_atexit);
  }
  
  int pager_in_use(void)
@@@ -150,8 -160,7 +160,8 @@@ int check_pager_config(const char *cmd
  	struct strbuf key = STRBUF_INIT;
  	const char *value = NULL;
  	strbuf_addf(&key, "pager.%s", cmd);
 -	if (!git_config_get_value(key.buf, &value)) {
 +	if (git_config_key_is_valid(key.buf) &&
 +	    !git_config_get_value(key.buf, &value)) {
  		int b = git_config_maybe_bool(key.buf, value);
  		if (b >= 0)
  			want = b;
diff --combined run-command.c
index c8029f2394,fe116bc2b1..e17e456cda
--- a/run-command.c
+++ b/run-command.c
@@@ -18,26 -18,27 +18,27 @@@ struct child_to_clean 
  static struct child_to_clean *children_to_clean;
  static int installed_child_cleanup_handler;
  
- static void cleanup_children(int sig)
+ static void cleanup_children(int sig, int in_signal)
  {
  	while (children_to_clean) {
  		struct child_to_clean *p = children_to_clean;
  		children_to_clean = p->next;
  		kill(p->pid, sig);
- 		free(p);
+ 		if (!in_signal)
+ 			free(p);
  	}
  }
  
  static void cleanup_children_on_signal(int sig)
  {
- 	cleanup_children(sig);
+ 	cleanup_children(sig, 1);
  	sigchain_pop(sig);
  	raise(sig);
  }
  
  static void cleanup_children_on_exit(void)
  {
- 	cleanup_children(SIGTERM);
+ 	cleanup_children(SIGTERM, 0);
  }
  
  static void mark_child_for_cleanup(pid_t pid)
@@@ -200,6 -201,7 +201,6 @@@ static int execv_shell_cmd(const char *
  #endif
  
  #ifndef GIT_WINDOWS_NATIVE
 -static int child_err = 2;
  static int child_notifier = -1;
  
  static void notify_parent(void)
@@@ -211,6 -213,17 +212,6 @@@
  	 */
  	xwrite(child_notifier, "", 1);
  }
 -
 -static NORETURN void die_child(const char *err, va_list params)
 -{
 -	vwritef(child_err, "fatal: ", err, params);
 -	exit(128);
 -}
 -
 -static void error_child(const char *err, va_list params)
 -{
 -	vwritef(child_err, "error: ", err, params);
 -}
  #endif
  
  static inline void set_cloexec(int fd)
@@@ -220,7 -233,7 +221,7 @@@
  		fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
  }
  
- static int wait_or_whine(pid_t pid, const char *argv0)
+ static int wait_or_whine(pid_t pid, const char *argv0, int in_signal)
  {
  	int status, code = -1;
  	pid_t waiting;
@@@ -228,6 -241,8 +229,8 @@@
  
  	while ((waiting = waitpid(pid, &status, 0)) < 0 && errno == EINTR)
  		;	/* nothing */
+ 	if (in_signal)
+ 		return 0;
  
  	if (waiting < 0) {
  		failed_errno = errno;
@@@ -350,10 -365,11 +353,10 @@@ fail_pipe
  		 * in subsequent call paths use the parent's stderr.
  		 */
  		if (cmd->no_stderr || need_err) {
 -			child_err = dup(2);
 +			int child_err = dup(2);
  			set_cloexec(child_err);
 +			set_error_handle(fdopen(child_err, "w"));
  		}
 -		set_die_routine(die_child);
 -		set_error_routine(error_child);
  
  		close(notify_pipe[0]);
  		set_cloexec(notify_pipe[1]);
@@@ -437,7 -453,7 +440,7 @@@
  		 * At this point we know that fork() succeeded, but execvp()
  		 * failed. Errors have been reported to our stderr.
  		 */
- 		wait_or_whine(cmd->pid, cmd->argv[0]);
+ 		wait_or_whine(cmd->pid, cmd->argv[0], 0);
  		failed_errno = errno;
  		cmd->pid = -1;
  	}
@@@ -536,12 -552,18 +539,18 @@@
  
  int finish_command(struct child_process *cmd)
  {
- 	int ret = wait_or_whine(cmd->pid, cmd->argv[0]);
+ 	int ret = wait_or_whine(cmd->pid, cmd->argv[0], 0);
  	argv_array_clear(&cmd->args);
  	argv_array_clear(&cmd->env_array);
  	return ret;
  }
  
+ int finish_command_in_signal(struct child_process *cmd)
+ {
+ 	return wait_or_whine(cmd->pid, cmd->argv[0], 1);
+ }
+ 
+ 
  int run_command(struct child_process *cmd)
  {
  	int code;
@@@ -595,7 -617,7 +604,7 @@@ static NORETURN void die_async(const ch
  {
  	vreportf("fatal: ", err, params);
  
 -	if (!pthread_equal(main_thread, pthread_self())) {
 +	if (in_async()) {
  		struct async *async = pthread_getspecific(async_key);
  		if (async->proc_in >= 0)
  			close(async->proc_in);
@@@ -614,13 -636,6 +623,13 @@@ static int async_die_is_recursing(void
  	return ret != NULL;
  }
  
 +int in_async(void)
 +{
 +	if (!main_thread_set)
 +		return 0; /* no asyncs started yet */
 +	return !pthread_equal(main_thread, pthread_self());
 +}
 +
  #else
  
  static struct {
@@@ -660,12 -675,6 +669,12 @@@ int git_atexit(void (*handler)(void)
  }
  #define atexit git_atexit
  
 +static int process_is_async;
 +int in_async(void)
 +{
 +	return process_is_async;
 +}
 +
  #endif
  
  int start_async(struct async *async)
@@@ -725,7 -734,6 +734,7 @@@
  		if (need_out)
  			close(fdout[0]);
  		git_atexit_clear();
 +		process_is_async = 1;
  		exit(!!async->proc(proc_in, proc_out, async->data));
  	}
  
@@@ -786,7 -794,7 +795,7 @@@ error
  int finish_async(struct async *async)
  {
  #ifdef NO_PTHREADS
- 	return wait_or_whine(async->pid, "child process");
+ 	return wait_or_whine(async->pid, "child process", 0);
  #else
  	void *ret = (void *)(intptr_t)(-1);
  
@@@ -798,13 -806,11 +807,13 @@@
  
  const char *find_hook(const char *name)
  {
 -	const char *path = git_path("hooks/%s", name);
 -	if (access(path, X_OK) < 0)
 -		path = NULL;
 +	static struct strbuf path = STRBUF_INIT;
  
 -	return path;
 +	strbuf_reset(&path);
 +	strbuf_git_path(&path, "hooks/%s", name);
 +	if (access(path.buf, X_OK) < 0)
 +		return NULL;
 +	return path.buf;
  }
  
  int run_hook_ve(const char *const *env, const char *name, va_list args)
diff --combined run-command.h
index 629fab7ae0,518663eef5..5428b048e2
--- a/run-command.h
+++ b/run-command.h
@@@ -50,13 -50,9 +50,14 @@@ void child_process_init(struct child_pr
  
  int start_command(struct child_process *);
  int finish_command(struct child_process *);
+ int finish_command_in_signal(struct child_process *);
  int run_command(struct child_process *);
  
 +/*
 + * Returns the path to the hook file, or NULL if the hook is missing
 + * or disabled. Note that this points to static storage that will be
 + * overwritten by further calls to find_hook and run_hook_*.
 + */
  extern const char *find_hook(const char *name);
  LAST_ARG_MUST_BE_NULL
  extern int run_hook_le(const char *const *env, const char *name, ...);
@@@ -118,6 -114,5 +119,6 @@@ struct async 
  
  int start_async(struct async *async);
  int finish_async(struct async *async);
 +int in_async(void);
  
  #endif