Re: [PATCH] http-push: fix webdav lock leak.

["Grégoire Barbier" <gb@gbarbier.org>]

Johannes Schindelin a écrit :
> > $gmane/70407 <1200250979-19604-2-git-send-email-gb@gbarbier.org>
>
>  I first could not reproduce the breakage described in the commit
>  message (bad or no ref given on command line).

It's rather easy anyway:

First, you need a test git repository availlable over http+webdav, let's 
say at http://myhost/myrepo.git/
Then, you do this:
$ git clone http://myhost/myrepo.git/
$ cd myrepo
$ git push http
Fetching remote heads...
  refs/
  refs/heads/
  refs/tags/
No refs in common and none specified; doing nothing.
$ git push http
Fetching remote heads...
  refs/
  refs/heads/
  refs/tags/
No refs in common and none specified; doing nothing.
$

Finally, you look at the web server logs, and will find one LOCK query 
and no UNLOCK query, of course the second one will be in 423 return code 
instead of 200:
1.2.3.4 - gb [19/Jan/2008:14:24:56 +0100] "LOCK /myrepo.git/info/refs 
HTTP/1.1" 200 465
(...)
1.2.3.4 - gb [19/Jan/2008:14:25:10 +0100] "LOCK /myrepo.git/info/refs 
HTTP/1.1" 423 363

With my patch, there would have be two UNLOCKs in addition of the LOCKs

 From the user point of view:
- If you realize that you should have typed e.g. "git push http master" 
instead of "git push http", you will have to wait for 10 minutes for the 
lock to expire by its own.
- Furthermore, if somebody else is dumb enough to type "git push http" 
while you need to push "master" branch, then you'll need too to wait for 
10 minutes too.

>  After playing around for a while, all of a sudden, I got a
>  segmentation fault:
>
>  Waiting for
> 
http://dscho@127.0.0.1/test.git/objects/56/5e84516c1c6dca168be1715b45aeae70b24d13_36e8d912-4841-455a-bbd9-69e54d00db99
>  Segmentation fault (core dumped)
>
>  Unfortunately, this is with _and_ without this patch.
>
>  In gdb, it looks like this:
(...)
>  The segmentation fault occurs due to check_locks() accessing the
>  remote that was just free()d, due to the new change.
>
>  But now I cannot even reproduce the segmentation fault, it seems.
>  Strange.  Very strange.
>
>  Grégoire, it would help tremendously if you could come up with a test
>  case.  The description you gave did not lead to something
>  reproducible here.

I don't know what's wrong but I can't manage to reproduce the segfault, 
I'm using the master branch on git.git plus my patches, and with CFLAGS 
containing -DUSE_CURL_MULTI, nothing more nothing less.
Is the test case I described above is enough for you to make another test?
What kind of additional information would you need ?

I will resubmit this patch today with a more detailled commit message 
including the way to reproduce the issue.
BTW you'll be interested to look at one of the "patches" I will repost 
today, since it's related to this one (the patch subject is "http-push: 
fail when info/refs exists and is already locked").

-- 
Grégoire Barbier - gb à gbarbier.org - +33 6 21 35 73 49