Merge branch 'rs/daemon-plug-child-leak' into maint
authorJunio C Hamano <gitster@pobox.com>
Thu, 5 Nov 2015 20:18:16 +0000 (12:18 -0800)
committerJunio C Hamano <gitster@pobox.com>
Thu, 5 Nov 2015 20:18:17 +0000 (12:18 -0800)
"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()

1  2 
daemon.c
run-command.c
run-command.h
diff --combined daemon.c
index f9eb296888c37c8f2a523f3941be027ddb36df3c,385934d46e8bc8c76f0eb86824f0f879bf0a0d77..77a2f03865ee84792d8c4316ceabcda1e075f1cd
+++ 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 e09275bd9e360ee7683ef5d3753ccfe753ee8dcc,fc391fb9cdd6977e8e40d00d2c6142fb824f99d1..84e4ce66e9d3da22be318813d75cbb3a94b614e4
@@@ -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;
  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)
         */
        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)
                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;
  
        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;
                }
                 * 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]);
                 * 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;
        }
                        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;
        }
  
  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);
  
  #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 275d35c442ac575f8f3f40aeb915cd0d341c776f,c850a57ea6bae33a164af135df2f1edfb0118f4a..f315868a03b0969f5ad111510df1c17c0c32b702
@@@ -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);