Is anybody else seeing segfaults on dumb HTTP pull with USE_CURL_MULTI? For example, this crashes for me:
$ git clone http://git.enyo.de/fw/debian/debfoster.git upstream
GDB shows that this happens inside the call to curl_multi_perform.
Sean[PATCH] Remove possible segfault in http-fetch.
Free the curl string lists after running http_cleanup to avoid an occasional segfault in the curl library. Seems to only occur if the website returns a 405 error.
Signed-off-by: Sean Estabrooks <seanlkml@sympatico.ca> ---
On Sat, 20 May 2006 20:47:54 +0200 Florian Weimer <fw@deneb.enyo.de> wrote:
Show 7 quoted lines
> Is anybody else seeing segfaults on dumb HTTP pull with > USE_CURL_MULTI? For example, this crashes for me: > > $ git clone http://git.enyo.de/fw/debian/debfoster.git upstream > > GDB shows that this happens inside the call to curl_multi_perform. >
Florian, could you please test this patch.
It comes with a big disclaimer because I don't really know the code in here all that well. However gdb reports the segfault happens in a strncasecmp call, and seeing as we've released a bunch of strings prior to the call....
Testing seems to confirm that the segfault is removed by this patch.
As to why the website returns a 405 error in the first place is still a mystery to me.
Sean
http-fetch.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/http-fetch.c b/http-fetch.c index 861644b..178f1ee 100644 --- a/http-fetch.c +++ b/http-fetch.c @@ -1269,10 +1269,10 @@ int main(int argc, char **argv) if (pull(commit_id)) rc = 1; - curl_slist_free_all(no_pragma_header); - http_cleanup(); + curl_slist_free_all(no_pragma_header); + if (corrupt_object_found) { fprintf(stderr, "Some loose object were found to be corrupt, but they might be just\n"
-- 1.3.GIT
Re: [PATCH] Remove possible segfault in http-fetch.
* Sean:
> Testing seems to confirm that the segfault is removed by this patch.
It seems to fix it for me, too.
> As to why the website returns a 405 error in the first place is still > a mystery to me.
The web server does not support PROPFIND.
Junio C HamanoRe: [PATCH] Remove possible segfault in http-fetch.
Sean <seanlkml@sympatico.ca> writes:
Show 29 quoted lines
> Free the curl string lists after running http_cleanup to
> avoid an occasional segfault in the curl library. Seems
> to only occur if the website returns a 405 error.
>...
> It comes with a big disclaimer because I don't really know the
> code in here all that well. However gdb reports the segfault
> happens in a strncasecmp call, and seeing as we've released a
> bunch of strings prior to the call....
>
> http-fetch.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/http-fetch.c b/http-fetch.c
> index 861644b..178f1ee 100644
> --- a/http-fetch.c
> +++ b/http-fetch.c
> @@ -1269,10 +1269,10 @@ int main(int argc, char **argv)
> if (pull(commit_id))
> rc = 1;
>
> - curl_slist_free_all(no_pragma_header);
> -
> http_cleanup();
>
> + curl_slist_free_all(no_pragma_header);
> +
> if (corrupt_object_found) {
> fprintf(stderr,
> "Some loose object were found to be corrupt, but they might be just\n"curl_easy_cleanup() which is called from http_cleanup() says it is safe to remove the strings _after_ you call that function, so I think the change makes sense -- it was apparently unsafe to free them before calling cleanup.
Knowing nothing about quirks in curl libraries, one thing that is mystery to me is that we slist_append() to other two lists (pragma_header and range_header) but we do not seem to ever free them. Another slist dav_headers is allocated and then freed inside a function, so that call-pattern seems well-formed.
Nick, care to help us out?
Re: [PATCH] Remove possible segfault in http-fetch.
On Sun, May 21, 2006 at 12:49:19AM -0700, Junio C Hamano wrote:
Show 12 quoted lines
> curl_easy_cleanup() which is called from http_cleanup() says it > is safe to remove the strings _after_ you call that function, so > I think the change makes sense -- it was apparently unsafe to > free them before calling cleanup. > > Knowing nothing about quirks in curl libraries, one thing that > is mystery to me is that we slist_append() to other two lists > (pragma_header and range_header) but we do not seem to ever free > them. Another slist dav_headers is allocated and then freed > inside a function, so that call-pattern seems well-formed. > > Nick, care to help us out?
I just got back from a trip to the midwest and am still getting caught up. I was only gone for 10 days, you've all been quite busy...
You're correct wrt the other slists, I'll get to work on a patch for that after I've caught up.
I'm also doing additional testing to see whether this fixes the DAV/405 segfault as I think there may be something else going on there.
-- For a successful technology, reality must take precedence over public relations, for nature cannot be fooled.
Re: Segfaults with USE_CURL_MULTI
On Sat, 2006-05-20 at 20:47 +0200, Florian Weimer wrote:
> Is anybody else seeing segfaults on dumb HTTP pull with > USE_CURL_MULTI?
_Everybody_ is seeing them! Just look for "segfaults" in the archives:
http://marc.theaimsgroup.com/?l=git&w=2&r=1&s=segfaults&q=b
This patch looks promising, but I'm yet to test it: http://marc.theaimsgroup.com/?l=git&m=114816558325617&w=2
Show 5 quoted lines
> For example, this crashes for me: > > $ git clone http://git.enyo.de/fw/debian/debfoster.git upstream > > GDB shows that this happens inside the call to curl_multi_perform.
Same for me: http://marc.theaimsgroup.com/?l=git&m=114790994726570&w=2
-- Regards, Pavel Roskin