{"thread":{"id":"23136","subject":"git clone over smart-http hanging for just one repo.","startedAt":"2010-03-22T06:52:43Z","lastAt":"2010-03-24T21:37:44Z","messageCount":9,"participants":["Brady Catherman","Shawn O. Pearce","Junio C Hamano","Nicolas Pitre","Shawn Pearce"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"137522","messageId":"51569EE6-A926-45DB-A588-B659750BA643@catherman.org","threadId":"23136","inReplyTo":null,"subject":"git clone over smart-http hanging for just one repo.","fromName":"Brady Catherman","fromEmail":"brady@catherman.org","sentAt":"2010-03-22T06:52:43Z","receivedAt":"2010-03-22T06:52:43Z","isPatch":false,"sender":{"key":"brady@catherman.org","avatar":null},"body":"I have a git repo that fails to clone or fetch over smart-http, but \nworks great over dav. I am wondering if somebody can help me debug the \nissue since I am at a loss why this is happening.\n\nJust for clarification, I have several dozen repos in a directory, \nalmost all others work without issue, but one of them is messed up. \nReplacing it with a different repo works so the error has to be \nsomewhere in the git files themselves. Cloning the repo via DAV works \nand git fsck reports no problems with the repo.\n\nThis is all on git 1.7.0.1 on a Linux machine.\n\nThe interesting parts of a strace of git-http-backend following a git \nclone follow:\n\n12037 execve(\"/usr/libexec/git-core/git-http-backend.real\", [\"/usr/ \nlibexec/git-core/git-http-backend.real\"], [/* 33 vars */]) = 0\n... various library loads and such ...\n12037 access(\"/usr/local/git/gitrepo\", F_OK) = 0\n12037 chdir(\"/usr/local/git/gitrepo\") = 0\n12037 access(\"objects\", X_OK) = 0\n12037 access(\"refs\", X_OK) = 0\n12037 lstat(\"HEAD\", {st_mode=S_IFREG|0644, st_size=23, ...}) = 0\n12037 open(\"HEAD\", O_RDONLY) = 3\n12037 read(3, \"ref: refs/heads/master\\n\", 255) = 23\n12037 access(\"/usr/local/etc/gitconfig\", R_OK) = -1 ENOENT (No such \nfile or directory)\n12037 access(\"./config\", R_OK) = 0\n12037 open(\"./config\", O_RDONLY) = 3\n12037 read(3, \"[core]\\n\\trepositoryformatversion = 0\\n\\tfilemode = true \n\\n\\tbare = true\\n\\tlogallrefupdates = true\\n\\tsharedRepository = true \n\\n\", 4096) = 116\n12037 read(3, \"\", 4096) = 0\n12037 close(3) = 0\n12037 munmap(0x2aaaaaaac000, 4096) = 0\n12037 write(1, \"Expires: Fri, 01 Jan 1980 00:00:00 GMT\\r\\n\", 40) = 40\n12037 write(1, \"Pragma: no-cache\\r\\n\", 18) = 18\n12037 write(1, \"Cache-Control: no-cache, max-age=0, must-revalidate\\r \n\\n\", 53) = 53\n12037 write(1, \"Content-Type: application/x-git-upload-pack-result\\r \n\\n\", 52) = 52\n12037 write(1, \"\\r\\n\", 2) = 2\n12037 pipe([3, 4]) = 0\n12037 pipe([3, 4]) = 012037 pipe([5, \n6]) = 0\n12037 clone(child_stack=0, flags=CLONE_CHILD_CLEARTID| \nCLONE_CHILD_SETTID|SIGCHLD\n, child_tidptr=0x2aaaaae12c20) = 12038\n12038 close(5 \n12037 close(6) = 0\n12037 read(5, \n12038 <... close resumed> ) = 0\n12038 fcntl(6, F_GETFD) = 0\n12038 fcntl(6, F_SETFD, FD_CLOEXEC) = 012038 dup2(3, \n0) = 0\n12038 close(3) = 012038 \nclose(4) = 0\n12038 execve(\"/usr/local/libexec/git-core/git\", [\"git\", \"upload-pack\", \n\"--stateless-rpc\", \".\"], [/* 36 vars */]) = -1 ENOENT (No such file or \ndirectory)\n12038 execve(\"/usr/libexec/git-core/git\", [\"git\", \"upload-pack\", \"-- \nstateless-rp\nc\", \".\"], [/* 36 vars */] \n12037 <... read resumed> \"\", 1) = 0\n12037 <... read resumed> \"\", 1) = 012037 \nclose(5) = 0\n12037 close(3) = 012038 <... execve \nresumed> ) = 0\n12037 close(1) = 0\n... More random library loads ...\n12038 pipe([3, 4]) = 0\n12038 clone(child_stack=0, flags=CLONE_CHILD_CLEARTID| \nCLONE_CHILD_SETTID|SIGCHLD, child_tidptr=0x2aaaaae12c40) = 12039\n12039 close(3) = 0\n12039 fcntl(4, F_GETFD) = 0\n12039 fcntl(4, F_SETFD, FD_CLOEXEC) = 0\n12039 execve(\"/usr/libexec/git-core/git-upload-pack\", [\"git-upload- \npack\", \"--stateless-rpc\", \".\"], [/* 36 vars */] \n12038 close(4) = 0\n12037 read(0, \"0067want c83f5a4ec9fb6b6681e74dc3c2276de5b947a76c \nmulti_ack_detailed side-band-64k thin-pack ofs-delta\\n0032want \n92e9b73bbd59b5ecf711381716c8aa13948f5a5d\\n0032want\n.. clipped out lots of sha's ..\n98040a8100513ad2852ca911ac330ebd4ff9e10d\\n00000009done\\n\", 8192) = 2316\n12039 <... execve resumed> ) = 0\n12038 <... read resumed> \"\", 1) = 0\n12038 close(3) = 0\n12039 brk(0 \n12038 wait4(12039, \n12039 <... brk resumed> ) = 0xf9d1000\n12037 write(1, \"Status: 500 Internal Server Error\\r\\n\", 35 \n\n12039 mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE| \nMAP_ANONYMOUS, -1, 0 \n12037 <... write resumed> ) = -1 EBADF (Bad file descriptor)\n12039 <... mmap resumed> ) = 0x2aaaaaaab000\n... This repeats for a while.. spitting out 500's on a bad file \ndescriptor...\n12037 write(1, \"Status: 500 Internal Server Error\\r\\n\", 35) = -1 EBADF \n(Bad file descriptor)\n... Eventually this just repeats a few thousand times ...\n12037 write(1, \"Status: 500 Internal Server Error\\r\\n\", 35) = -1 EBADF \n(Bad file descriptor)\n12037 --- SIGSEGV (Segmentation fault) @ 0 (0) ---\n12037 +++ killed by SIGSEGV +++\n12039 <... read resumed> \"\", 4) = 0\n12039 write(2, \"fatal: The remote end hung up unexpectedly\\n\", 43) = 43\n12039 exit_group(128) = ?\n12038 <... wait4 resumed> [{WIFEXITED(s) && WEXITSTATUS(s) == 128}], \n0, NULL) = 12039\n12038 --- SIGCHLD (Child exited) @ 0 (0) ---\n12038 exit_group(128) = ?\n\nAnybody have any thoughts why this would happen or what can be done to \nfix it?\n"},{"id":"137550","messageId":"20100322141230.GA8916@spearce.org","threadId":"23136","inReplyTo":"51569EE6-A926-45DB-A588-B659750BA643@catherman.org","subject":"Re: git clone over smart-http hanging for just one repo.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2010-03-22T14:12:30Z","receivedAt":"2010-03-22T14:12:30Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Brady Catherman wrote:\n> I have a git repo that fails to clone or fetch over smart-http, but \n> works great over dav. I am wondering if somebody can help me debug the \n> issue since I am at a loss why this is happening.\n\nYea, I'm at a loss too. :-(\n\n> The interesting parts of a strace of git-http-backend following a git \n> clone follow:\n...\n> 12037 close(1) = 0\n\nWhy did the CGI process just close stdout? I'm guessing this is\npart of the exec of the upload-pack child in the background. Oh,\nright, we closed it because we passed the descriptor to the child\nand now the parent CGI doesn't want it anymore.\n\n> 12037 write(1, \"Status: 500 Internal Server Error\\r\\n\", 35) = -1 EBADF \n> (Bad file descriptor)\n\nThis smells like the backend upload-pack process got into trouble and\nexited early, so now the CGI is trying to change the status to 500\nsince the backend exited with a non-zero status. Only its too late,\nas the filedescriptor was already closed after the successful fork().\n\nWe're stuck in a loop because we're failing during the die routine.\n\nBecause the file descriptor is closed, safe_write() which was what\noriginated that write(1, ...) above, tries to call die(). But that\ndie() call invokes die_webcgi() which in turn tries to write that\n500 error message again to 1. So this goes on a for a while...\n\n> 12037 --- SIGSEGV (Segmentation fault) @ 0 (0) ---\n> 12037 +++ killed by SIGSEGV +++\n\nand then we run out of stack space, due to too many recursions,\nand the process is aborted by a SIGSEGV.\n\n> Anybody have any thoughts why this would happen or what can be done to \n> fix it?\n\nA gdb trace or something of the upload-pack process would help.\nThat appears to have also died and we don't know why. Its death\nis what contributed to the CGI crashing above.\n\nI'll try to send a patch for this recursive crashing problem in\nthe CGI.\n\n-- \nShawn.\n"},{"id":"137552","messageId":"20100322142204.GB8916@spearce.org","threadId":"23136","inReplyTo":"51569EE6-A926-45DB-A588-B659750BA643@catherman.org","subject":"[PATCH] http-backend: Don't infinite loop during die()","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2010-03-22T14:22:04Z","receivedAt":"2010-03-22T14:22:04Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"If stdout has already been closed by the CGI and die() gets called,\nthe CGI will fail to write the \"Status: 500 Internal Server Error\" to\nthe pipe, which results in die() being called again (via safe_write).\nThis goes on in an infinite loop until the stack overflows and the\nprocess is killed by SIGSEGV.\n\nInstead set a flag on the first die() invocation and perform no\naction during recursive die() calls. This way failures to write the\nerror messages to the stdout pipe do not result in an infinite loop.\n\nWe also now report on the death to stderr before we report to stdout,\nto increase the chances that the cause of the die() invocation will\nappear in the server's error log.\n\nSigned-off-by: Shawn O. Pearce \n---\n\n This patch should be put in maint.\n\n It doesn't fix the underlying problem Brady has found, but it\n will at least get us more information by avoiding the infinite\n loop and later SIGSEGV crash of the parent CGI.\n\n http-backend.c | 15 ++++++++++-----\n 1 files changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/http-backend.c b/http-backend.c\nindex 8c7b7d0..5c0d649 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -538,12 +538,17 @@ static void service_rpc(char *service_name)\n \n static NORETURN void die_webcgi(const char *err, va_list params)\n {\n-\thttp_status(500, \"Internal Server Error\");\n-\thdr_nocache();\n-\tend_headers();\n+\tstatic int dead;\n \n-\tvreportf(\"fatal: \", err, params);\n-\texit(0);\n+\tif (!dead) {\n+\t\tdead = 1;\n+\n+\t\tvreportf(\"fatal: \", err, params);\n+\t\thttp_status(500, \"Internal Server Error\");\n+\t\thdr_nocache();\n+\t\tend_headers();\n+\t\texit(0);\n+\t}\n }\n \n static char* getdir(void)\n-- \n1.6.4.rc2.182.g24de1\n\n-- \nShawn.\n"},{"id":"137585","messageId":"7vhbo885os.fsf@alter.siamese.dyndns.org","threadId":"23136","inReplyTo":"20100322142204.GB8916@spearce.org","subject":"Re: [PATCH] http-backend: Don't infinite loop during die()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-22T20:25:23Z","receivedAt":"2010-03-22T20:25:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> If stdout has already been closed by the CGI and die() gets called,\n> the CGI will fail to write the \"Status: 500 Internal Server Error\" to\n> the pipe, which results in die() being called again (via safe_write).\n> This goes on in an infinite loop until the stack overflows and the\n> process is killed by SIGSEGV.\n\nBefore looking at the code I first thought \"wouldn't we have the same\nproblem in die(), and shouldn't we have \"dying\" flag there?\" but I was\nstupid as usual ;-)\n\nThe patch looks sane. Thanks.\n"},{"id":"137718","messageId":"7vvdclk1y9.fsf@alter.siamese.dyndns.org","threadId":"23136","inReplyTo":"20100322142204.GB8916@spearce.org","subject":"Re: [PATCH] http-backend: Don't infinite loop during die()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-24T18:29:50Z","receivedAt":"2010-03-24T18:29:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> If stdout has already been closed by the CGI and die() gets called,\n> the CGI will fail to write the \"Status: 500 Internal Server Error\" to\n> the pipe, which results in die() being called again (via safe_write).\n> This goes on in an infinite loop until the stack overflows and the\n> process is killed by SIGSEGV.\n>\n> Instead set a flag on the first die() invocation and perform no\n> action during recursive die() calls. This way failures to write the\n> error messages to the stdout pipe do not result in an infinite loop.\n\nHmm. I would need something like this on top, but there must be a better\nway. Ideas?\n\n-- >8 --\nSubject: [PATCH] fixup! http-backend.c: Don't infinite loop\n\nNow die_webcgi() actually can return during a recursive call into it,\ncausing\n\n http-backend.c:554: error: 'noreturn' function does return\n\nWork it around with a somewhat ugly workaround.\n\nSigned-off-by: Junio C Hamano \n---\n http-backend.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/http-backend.c b/http-backend.c\nindex f4d49b6..d3ec6f0 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -536,7 +536,7 @@ static void service_rpc(char *service_name)\n \tstrbuf_release(&buf);\n }\n \n-static NORETURN void die_webcgi(const char *err, va_list params)\n+static void die_webcgi(const char *err, va_list params)\n {\n \tstatic int dead;\n \n@@ -606,7 +606,7 @@ int main(int argc, char **argv)\n \tint i;\n \n \tgit_extract_argv0_path(argv[0]);\n-\tset_die_routine(die_webcgi);\n+\tset_die_routine((void *)die_webcgi);\n \n \tif (!method)\n \t\tdie(\"No REQUEST_METHOD from server\");\n-- \n1.7.0.3.435.g097f4\n"},{"id":"137731","messageId":"20100324200658.GA22505@spearce.org","threadId":"23136","inReplyTo":"7vvdclk1y9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] http-backend: Don't infinite loop during die()","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2010-03-24T20:06:58Z","receivedAt":"2010-03-24T20:06:58Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> \"Shawn O. Pearce\" writes:\n> \n> > If stdout has already been closed by the CGI and die() gets called,\n> > the CGI will fail to write the \"Status: 500 Internal Server Error\" to\n> > the pipe, which results in die() being called again (via safe_write).\n> > This goes on in an infinite loop until the stack overflows and the\n> > process is killed by SIGSEGV.\n> >\n> > Instead set a flag on the first die() invocation and perform no\n> > action during recursive die() calls. This way failures to write the\n> > error messages to the stdout pipe do not result in an infinite loop.\n> \n> Hmm. I would need something like this on top, but there must be a better\n> way. Ideas?\n\nIck.\n\nJust exit(0) if dead is true. The *only* reason we would come back\ninto the die handler at this point is because we failed during the\ndie handler. That recursive failure means we can't report the\nerrors, because that's all that was left once the dead variable\nwas set.\n\n-- \nShawn.\n"},{"id":"137735","messageId":"alpine.LFD.2.00.1003241623370.694@xanadu.home","threadId":"23136","inReplyTo":"20100324200658.GA22505@spearce.org","subject":"Re: [PATCH] http-backend: Don't infinite loop during die()","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2010-03-24T20:24:18Z","receivedAt":"2010-03-24T20:24:18Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 24 Mar 2010, Shawn O. Pearce wrote:\n\n> Junio C Hamano wrote:\n> > \"Shawn O. Pearce\" writes:\n> > \n> > > If stdout has already been closed by the CGI and die() gets called,\n> > > the CGI will fail to write the \"Status: 500 Internal Server Error\" to\n> > > the pipe, which results in die() being called again (via safe_write).\n> > > This goes on in an infinite loop until the stack overflows and the\n> > > process is killed by SIGSEGV.\n> > >\n> > > Instead set a flag on the first die() invocation and perform no\n> > > action during recursive die() calls. This way failures to write the\n> > > error messages to the stdout pipe do not result in an infinite loop.\n> > \n> > Hmm. I would need something like this on top, but there must be a better\n> > way. Ideas?\n> \n> Ick.\n> \n> Just exit(0) if dead is true.\n\nDo you really want the exit code to be 0 if you're dying?\n\n\nNicolas\n"},{"id":"137736","messageId":"ec874dac1003241325y43fdec78gfbb3d115e1aaa1b0@mail.gmail.com","threadId":"23136","inReplyTo":"alpine.LFD.2.00.1003241623370.694@xanadu.home","subject":"Re: [PATCH] http-backend: Don't infinite loop during die()","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2010-03-24T20:25:36Z","receivedAt":"2010-03-24T20:25:36Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"On Wed, Mar 24, 2010 at 1:24 PM, Nicolas Pitre wrote:\n> On Wed, 24 Mar 2010, Shawn O. Pearce wrote:\n>\n>> Junio C Hamano wrote:\n>> > \"Shawn O. Pearce\" writes:\n>> >\n>> > > If stdout has already been closed by the CGI and die() gets called,\n>> > > the CGI will fail to write the \"Status: 500 Internal Server Error\" to\n>> > > the pipe, which results in die() being called again (via safe_write).\n>> > > This goes on in an infinite loop until the stack overflows and the\n>> > > process is killed by SIGSEGV.\n>> > >\n>> > > Instead set a flag on the first die() invocation and perform no\n>> > > action during recursive die() calls.  This way failures to write the\n>> > > error messages to the stdout pipe do not result in an infinite loop.\n>> >\n>> > Hmm.  I would need something like this on top, but there must be a better\n>> > way.  Ideas?\n>>\n>> Ick.\n>>\n>> Just exit(0) if dead is true.\n>\n> Do you really want the exit code to be 0 if you're dying?\n\nIIRC, yes. If we exit non-zero Apache really freaked out. That was a\nbig part of my motivation to write the die_webcgi handler.\n\n-- \nShawn.\n"},{"id":"137749","messageId":"7veij91jvb.fsf@alter.siamese.dyndns.org","threadId":"23136","inReplyTo":"20100324200658.GA22505@spearce.org","subject":"Re: [PATCH] http-backend: Don't infinite loop during die()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-24T21:37:44Z","receivedAt":"2010-03-24T21:37:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> Just exit(0) if dead is true.\n\nheh, very true.\n\n-- >8 --\nSubject: [PATCH] fixup! http-backend.c: Don't infinite loop\n\nNow die_webcgi() actually can return during a recursive call into it,\ncausing\n\n http-backend.c:554: error: 'noreturn' function does return\n\nThe only reason we would come back to the die handler is because we\nfailed during it, so we cannot report anything anyway.\n\nSigned-off-by: Junio C Hamano \n---\n http-backend.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/http-backend.c b/http-backend.c\nindex f4d49b6..5e3f277 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -549,8 +549,8 @@ static NORETURN void die_webcgi(const char *err, va_list params)\n \t\thttp_status(500, \"Internal Server Error\");\n \t\thdr_nocache();\n \t\tend_headers();\n-\t\texit(0);\n \t}\n+\texit(0); /* keep apache happy */\n }\n \n static char* getdir(void)\n"}]}