On Sun, Apr 24, 2011 at 10:42:20PM +0200, Johannes Sixt wrote: > In the non-stateless-rpc case, the writable end of the channel to the > remote repo is closed by the start_command() call that runs the > pack-objects process (after pack-objects inherited a copy). But in the > --stateless-rpc case, where send-pack takes care of writing data to the > channel, this was missed. > > [...] > > Warning: This patch is untested. Furthermore, it does not even fix a resource > leak because the fd that is now closed in pack_objects() would be closed > later in cmd_send_pack. Note that we can also call send_pack directly from git-push via the transport.c interface. I didn't check whether one can actually trigger stateless-rpc this way, though; it looks like git-remote-http ends up exec'ing a separate send-pack. > However, without closing the fd earlier like this, a --stateless-rpc > invocation could theoretically dead-lock just like a regular > invocation in a NO_PTHREADS build. But I also don't know how to > test-drive send-pack --stateless-rpc to construct such a case. Any > hints how to do that would be appreciated. I was able to get a hang using v1.7.5 compiled with pthreads. You need to have a server accepting smart-http push (if you use github, you can create an empty test repo there, which is sufficient). And then do a modified version of the test I posted earlier: UPSTREAM=https://peff@xxxxxxxxxx/peff/test.git git init child && cd child && git remote add origin $UPSTREAM && echo content >file && git add file && git commit -m one && echo content >>file && git add file && git commit -m two && sha1=`git rev-parse HEAD:file` && file=`echo $sha1 | sed 's,..,&/,'` && rm -fv .git/objects/$file where obviously you need to tweak $UPSTREAM to point to the repo you created. That sets up the broken repo state. You can then try "git push" in the repo with various versions to check their behavior. With stock v1.7.5, this will hang after pack-objects reports the fatal error. With your patch, it exits immediately, though the output looks like this: $ git push Password: Counting objects: 5, done. error: unable to find ea0faeb6073ff6cb085727c3647be457051e6ed7 fatal: unable to read ea0faeb6073ff6cb085727c3647be457051e6ed7 fatal: The remote end hung up unexpectedly fatal: The remote end hung up unexpectedly fatal: write error: Bad file descriptor which could perhaps be a little nicer, but is probably not a big deal (I didn't dig very deep, but presumably we should exit a little more immediately after seeing pack-objects fail). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html