Merge branch 'rs/daemon-plug-child-leak'
authorJunio C Hamano <gitster@pobox.com>
Tue, 3 Nov 2015 23:13:11 +0000 (15:13 -0800)
committerJunio C Hamano <gitster@pobox.com>
Tue, 3 Nov 2015 23:13:12 +0000 (15:13 -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 56679a15fee533a29168b92ecc0f18f68f919085,385934d46e8bc8c76f0eb86824f0f879bf0a0d77..be70cd4da09ccab8850be41e19b0fc4b9a10f502
+++ 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;
@@@ -811,6 -812,8 +812,6 @@@ static char **cld_argv
  static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen)
  {
        struct child_process cld = CHILD_PROCESS_INIT;
 -      char addrbuf[300] = "REMOTE_ADDR=", portbuf[300];
 -      char *env[] = { addrbuf, portbuf, NULL };
  
        if (max_connections && live_children >= max_connections) {
                kill_some_child();
        }
  
        if (addr->sa_family == AF_INET) {
 +              char buf[128] = "";
                struct sockaddr_in *sin_addr = (void *) addr;
 -              inet_ntop(addr->sa_family, &sin_addr->sin_addr, addrbuf + 12,
 -                  sizeof(addrbuf) - 12);
 -              snprintf(portbuf, sizeof(portbuf), "REMOTE_PORT=%d",
 -                  ntohs(sin_addr->sin_port));
 +              inet_ntop(addr->sa_family, &sin_addr->sin_addr, buf, sizeof(buf));
 +              argv_array_pushf(&cld.env_array, "REMOTE_ADDR=%s", buf);
 +              argv_array_pushf(&cld.env_array, "REMOTE_PORT=%d",
 +                               ntohs(sin_addr->sin_port));
  #ifndef NO_IPV6
        } else if (addr->sa_family == AF_INET6) {
 +              char buf[128] = "";
                struct sockaddr_in6 *sin6_addr = (void *) addr;
 -
 -              char *buf = addrbuf + 12;
 -              *buf++ = '['; *buf = '\0'; /* stpcpy() is cool */
 -              inet_ntop(AF_INET6, &sin6_addr->sin6_addr, buf,
 -                  sizeof(addrbuf) - 13);
 -              strcat(buf, "]");
 -
 -              snprintf(portbuf, sizeof(portbuf), "REMOTE_PORT=%d",
 -                  ntohs(sin6_addr->sin6_port));
 +              inet_ntop(AF_INET6, &sin6_addr->sin6_addr, buf, sizeof(buf));
 +              argv_array_pushf(&cld.env_array, "REMOTE_ADDR=[%s]", buf);
 +              argv_array_pushf(&cld.env_array, "REMOTE_PORT=%d",
 +                               ntohs(sin6_addr->sin6_port));
  #endif
        }
  
 -      cld.env = (const char **)env;
        cld.argv = (const char **)cld_argv;
        cld.in = incoming;
        cld.out = dup(incoming);
@@@ -895,7 -902,7 +896,7 @@@ static const char *ip2str(int family, s
                inet_ntop(family, &((struct sockaddr_in*)sin)->sin_addr, ip, len);
                break;
        default:
 -              strcpy(ip, "<unknown>");
 +              xsnprintf(ip, sizeof(ip), "<unknown>");
        }
        return ip;
  }
@@@ -910,7 -917,7 +911,7 @@@ static int setup_named_sock(char *liste
        int gai;
        long flags;
  
 -      sprintf(pbuf, "%d", listen_port);
 +      xsnprintf(pbuf, sizeof(pbuf), "%d", listen_port);
        memset(&hints, 0, sizeof(hints));
        hints.ai_family = AF_UNSPEC;
        hints.ai_socktype = SOCK_STREAM;
@@@ -1160,6 -1167,15 +1161,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)
  {
@@@ -1370,7 -1386,7 +1371,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 e17e456cdaef63d3fa587b8e216bf1fe5c372317,fc391fb9cdd6977e8e40d00d2c6142fb824f99d1..13fa452e8c3d5a5e20e24a969d53ce7ade4019bf
@@@ -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;
@@@ -604,7 -611,7 +607,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);
@@@ -623,13 -630,6 +626,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 {
@@@ -669,12 -669,6 +672,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)
                if (need_out)
                        close(fdout[0]);
                git_atexit_clear();
 +              process_is_async = 1;
                exit(!!async->proc(proc_in, proc_out, async->data));
        }
  
@@@ -795,7 -788,7 +798,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 5428b048e2c49dec3e322ac2a46ffddd012e4c8d,c850a57ea6bae33a164af135df2f1edfb0118f4a..12bb26c2a6155750203babfec47b08e8bde0ad27
@@@ -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);
@@@ -119,6 -114,5 +120,6 @@@ struct async 
  
  int start_async(struct async *async);
  int finish_async(struct async *async);
 +int in_async(void);
  
  #endif