From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 5 Nov 2015 20:18:16 +0000 (-0800)
Subject: Merge branch 'rs/daemon-plug-child-leak' into maint
X-Git-Tag: v2.6.3~1
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/b8f5242592146c85c2e09fd9721e522ce4dd32e5?ds=inline;hp=-c

Merge branch 'rs/daemon-plug-child-leak' into maint

"git daemon" uses "run_command()" without "finish_command()", so it
needs to release resources itself, which it forgot to do.

* rs/daemon-plug-child-leak:
daemon: plug memory leak
run-command: factor out child_process_clear()
---

b8f5242592146c85c2e09fd9721e522ce4dd32e5
diff --combined daemon.c
index f9eb296888,385934d46e..77a2f03865
--- a/daemon.c
+++ b/daemon.c
@@@ -802,6 -802,7 +802,7 @@@ static void check_dead_children(void
  			/* remove the child */
  			*cradle = blanket->next;
  			live_children--;
+ 			child_process_clear(&blanket->cld);
  			free(blanket);
  		} else
  			cradle = &blanket->next;
@@@ -1166,6 -1167,15 +1167,6 @@@ static struct credentials *prepare_cred
  }
  #endif
  
 -static void store_pid(const char *path)
 -{
 -	FILE *f = fopen(path, "w");
 -	if (!f)
 -		die_errno("cannot open pid file '%s'", path);
 -	if (fprintf(f, "%"PRIuMAX"\n", (uintmax_t) getpid()) < 0 || fclose(f) != 0)
 -		die_errno("failed to write pid file '%s'", path);
 -}
 -
  static int serve(struct string_list *listen_addr, int listen_port,
      struct credentials *cred)
  {
@@@ -1376,7 -1386,7 +1377,7 @@@ int main(int argc, char **argv
  		sanitize_stdfds();
  
  	if (pid_file)
 -		store_pid(pid_file);
 +		write_file(pid_file, "%"PRIuMAX, (uintmax_t) getpid());
  
  	/* prepare argv for serving-processes */
  	cld_argv = xmalloc(sizeof (char *) * (argc + 2));
diff --combined run-command.c
index e09275bd9e,fc391fb9cd..84e4ce66e9
--- a/run-command.c
+++ b/run-command.c
@@@ -11,6 -11,12 +11,12 @@@ void child_process_init(struct child_pr
  	argv_array_init(&child->env_array);
  }
  
+ void child_process_clear(struct child_process *child)
+ {
+ 	argv_array_clear(&child->args);
+ 	argv_array_clear(&child->env_array);
+ }
+ 
  struct child_to_clean {
  	pid_t pid;
  	struct child_to_clean *next;
@@@ -18,27 -24,26 +24,27 @@@
  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)
@@@ -201,6 -206,7 +207,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)
@@@ -212,6 -218,17 +218,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)
@@@ -221,7 -238,7 +227,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;
@@@ -229,8 -246,6 +235,8 @@@
  
  	while ((waiting = waitpid(pid, &status, 0)) < 0 && errno == EINTR)
  		;	/* nothing */
 +	if (in_signal)
 +		return 0;
  
  	if (waiting < 0) {
  		failed_errno = errno;
@@@ -327,8 -342,7 +333,7 @@@ int start_command(struct child_process 
  fail_pipe:
  			error("cannot create %s pipe for %s: %s",
  				str, cmd->argv[0], strerror(failed_errno));
- 			argv_array_clear(&cmd->args);
- 			argv_array_clear(&cmd->env_array);
+ 			child_process_clear(cmd);
  			errno = failed_errno;
  			return -1;
  		}
@@@ -353,10 -367,11 +358,10 @@@
  		 * 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]);
@@@ -440,7 -455,7 +445,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;
  	}
@@@ -513,8 -528,7 +518,7 @@@
  			close_pair(fderr);
  		else if (cmd->err)
  			close(cmd->err);
- 		argv_array_clear(&cmd->args);
- 		argv_array_clear(&cmd->env_array);
+ 		child_process_clear(cmd);
  		errno = failed_errno;
  		return -1;
  	}
@@@ -539,18 -553,11 +543,17 @@@
  
  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);
+ 	child_process_clear(cmd);
  	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;
@@@ -781,7 -788,7 +784,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);
  
@@@ -791,15 -798,13 +794,15 @@@
  #endif
  }
  
 -char *find_hook(const char *name)
 +const char *find_hook(const char *name)
  {
 -	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 275d35c442,c850a57ea6..f315868a03
--- a/run-command.h
+++ b/run-command.h
@@@ -47,18 -47,13 +47,19 @@@ struct child_process 
  
  #define CHILD_PROCESS_INIT { NULL, ARGV_ARRAY_INIT, ARGV_ARRAY_INIT }
  void child_process_init(struct child_process *);
+ void child_process_clear(struct child_process *);
  
  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 *);
  
 -extern char *find_hook(const char *name);
 +/*
 + * 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, ...);
  extern int run_hook_ve(const char *const *env, const char *name, va_list args);