From: Seth Jennings <spartacus06@gmail.com>
Date: Fri, 15 Apr 2011 16:54:12 GMT
Subject: Re: [PATCH] git-daemon: fix segfaulting in child_handler() in AIX
Message-ID: <BANLkTi=6UVWpa2aPPebVUG9ZyL_h7OcwUQ@mail.gmail.com>
In-Reply-To: <1302886260-25860-1-git-send-email-spartacus06@gmail.com>

There is a git-daemon segfault issue that seems to be specific to AIX.

Whenever a remote user pulls or clones, the operation succeeds but
git-daemon crashes immediately afterward.

$ gdb git-daemon core
...
Core was generated by `git-daemon'.
Program terminated with signal 11, Segmentation fault.
#0  0xd04f0c50 in _sigsetmask () from /usr/lib/libpthreads.a(shr_xpg5.o)
(gdb) where
#0  0xd04f0c50 in _sigsetmask () from /usr/lib/libpthreads.a(shr_xpg5.o)
#1  0xd04f1874 in _p_sigaction () from /usr/lib/libpthreads.a(shr_xpg5.o)
#2  0xd013ae34 in sigaction () from /usr/lib/libc.a(shr.o)
#3  0xd0217cd8 in signal () from /usr/lib/libc.a(shr.o)
#4  0x10000b90 in child_handler (signo=0) at daemon.c:718
#5  <signal handler called>

Through experimentation, I found that using sigaction() instead of
signal() resolves the issue.  I'm not entirely sure why this is.

Any feedback about the issue or the patch is welcome.  There might be
a better solution.

On Fri, Apr 15, 2011 at 11:51 AM, Seth Jennings <spartacus06@gmail.com> wrote:
> This issue seems to be specific to git-daemon on AIX built with xlc.
> After commit 695605b5080e1957bd9dab1fed35a7fee9814297 (from Aug 2008),
> git-daemon segfaults in child_handler() inside the signal() syscall
> immediately after any remote clone/pull operation.  While it is not
> fully understood why this happens, changing signal() to sigaction()
> resolves the issue.
>
> This commit converts singal() to sigaction() in child_handler().
> ---
>  daemon.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/daemon.c b/daemon.c
> index 4c8346d..3ea5b2c 100644
> --- a/daemon.c
> +++ b/daemon.c
> @@ -715,7 +715,10 @@ static void child_handler(int signo)
>         * upon signal receipt
>         * SysV needs the handler to be rearmed
>         */
> -       signal(SIGCHLD, child_handler);
> +       struct sigaction sigact;
> +       memset(&sigact, 0, sizeof(sigact));
> +       sigact.sa_handler = child_handler;
> +       sigaction(SIGCHLD, &sigact, NULL);
>  }
>
>  static int set_reuse_addr(int sockfd)
> @@ -889,7 +892,10 @@ static int service_loop(struct socketlist *socklist)
>                pfd[i].events = POLLIN;
>        }
>
> -       signal(SIGCHLD, child_handler);
> +       struct sigaction sigact;
> +       memset(&sigact, 0, sizeof(sigact));
> +       sigact.sa_handler = child_handler;
> +       sigaction(SIGCHLD, &sigact, NULL);
>
>        for (;;) {
>                int i;
> --
> 1.7.0.4
>
>