{"thread":{"id":"4220","subject":"Segfaults with USE_CURL_MULTI","startedAt":"2006-05-20T18:47:54Z","lastAt":"2006-05-31T16:07:31Z","messageCount":6,"participants":["Florian Weimer","Sean","Junio C Hamano","Pavel Roskin","Nick Hengeveld"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"20334","messageId":"87fyj4y1lx.fsf@mid.deneb.enyo.de","threadId":"4220","inReplyTo":null,"subject":"Segfaults with USE_CURL_MULTI","fromName":"Florian Weimer","fromEmail":"fw@deneb.enyo.de","sentAt":"2006-05-20T18:47:54Z","receivedAt":"2006-05-20T18:47:54Z","isPatch":false,"sender":{"key":"fw@deneb.enyo.de","avatar":null},"body":"Is anybody else seeing segfaults on dumb HTTP pull with\nUSE_CURL_MULTI? For example, this crashes for me:\n\n$ git clone http://git.enyo.de/fw/debian/debfoster.git upstream\n\nGDB shows that this happens inside the call to curl_multi_perform.\n"},{"id":"20344","messageId":"BAYC1-PASMTP082397700A9527CC2F3786AEA40@CEZ.ICE","threadId":"4220","inReplyTo":"87fyj4y1lx.fsf@mid.deneb.enyo.de","subject":"[PATCH] Remove possible segfault in http-fetch.","fromName":"Sean","fromEmail":"seanlkml@sympatico.ca","sentAt":"2006-05-20T22:46:33Z","receivedAt":"2006-05-20T22:46:33Z","isPatch":true,"sender":{"key":"seanlkml@sympatico.ca","avatar":"https://gravatar.com/avatar/f92923f54fc08c401fc59b71829d4b89e9b8087fbba45ff87c82e6a83aee02ae?d=mp&s=160"},"body":"\nFree the curl string lists after running http_cleanup to\navoid an occasional segfault in the curl library. Seems\nto only occur if the website returns a 405 error.\n\nSigned-off-by: Sean Estabrooks \n---\n\nOn Sat, 20 May 2006 20:47:54 +0200\nFlorian Weimer wrote:\n\n> Is anybody else seeing segfaults on dumb HTTP pull with\n> USE_CURL_MULTI? For example, this crashes for me:\n> \n> $ git clone http://git.enyo.de/fw/debian/debfoster.git upstream\n> \n> GDB shows that this happens inside the call to curl_multi_perform.\n> \n\nFlorian, could you please test this patch.\n\nIt comes with a big disclaimer because I don't really know the\ncode in here all that well. However gdb reports the segfault\nhappens in a strncasecmp call, and seeing as we've released a\nbunch of strings prior to the call....\n\nTesting seems to confirm that the segfault is removed by this patch.\n\nAs to why the website returns a 405 error in the first place is still\na mystery to me.\n\nSean\n\n\n http-fetch.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/http-fetch.c b/http-fetch.c\nindex 861644b..178f1ee 100644\n--- a/http-fetch.c\n+++ b/http-fetch.c\n@@ -1269,10 +1269,10 @@ int main(int argc, char **argv)\n \tif (pull(commit_id))\n \t\trc = 1;\n \n-\tcurl_slist_free_all(no_pragma_header);\n-\n \thttp_cleanup();\n \n+\tcurl_slist_free_all(no_pragma_header);\n+\n \tif (corrupt_object_found) {\n \t\tfprintf(stderr,\n \"Some loose object were found to be corrupt, but they might be just\\n\"\n-- \n1.3.GIT\n"},{"id":"20346","messageId":"87ejyonvx0.fsf@mid.deneb.enyo.de","threadId":"4220","inReplyTo":"BAYC1-PASMTP082397700A9527CC2F3786AEA40@CEZ.ICE","subject":"Re: [PATCH] Remove possible segfault in http-fetch.","fromName":"Florian Weimer","fromEmail":"fw@deneb.enyo.de","sentAt":"2006-05-20T23:00:59Z","receivedAt":"2006-05-20T23:00:59Z","isPatch":true,"sender":{"key":"fw@deneb.enyo.de","avatar":null},"body":"* Sean:\n\n> Testing seems to confirm that the segfault is removed by this patch.\n\nIt seems to fix it for me, too.\n\n> As to why the website returns a 405 error in the first place is still\n> a mystery to me.\n\nThe web server does not support PROPFIND.\n"},{"id":"20377","messageId":"7vverzzukg.fsf@assigned-by-dhcp.cox.net","threadId":"4220","inReplyTo":"BAYC1-PASMTP082397700A9527CC2F3786AEA40@CEZ.ICE","subject":"Re: [PATCH] Remove possible segfault in http-fetch.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-21T07:49:19Z","receivedAt":"2006-05-21T07:49:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sean writes:\n\n> Free the curl string lists after running http_cleanup to\n> avoid an occasional segfault in the curl library. Seems\n> to only occur if the website returns a 405 error.\n>...\n> It comes with a big disclaimer because I don't really know the\n> code in here all that well. However gdb reports the segfault\n> happens in a strncasecmp call, and seeing as we've released a\n> bunch of strings prior to the call....\n>\n> http-fetch.c | 4 ++--\n> 1 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/http-fetch.c b/http-fetch.c\n> index 861644b..178f1ee 100644\n> --- a/http-fetch.c\n> +++ b/http-fetch.c\n> @@ -1269,10 +1269,10 @@ int main(int argc, char **argv)\n> \tif (pull(commit_id))\n> \t\trc = 1;\n> \n> -\tcurl_slist_free_all(no_pragma_header);\n> -\n> \thttp_cleanup();\n> \n> +\tcurl_slist_free_all(no_pragma_header);\n> +\n> \tif (corrupt_object_found) {\n> \t\tfprintf(stderr,\n> \"Some loose object were found to be corrupt, but they might be just\\n\"\n\ncurl_easy_cleanup() which is called from http_cleanup() says it\nis safe to remove the strings _after_ you call that function, so\nI think the change makes sense -- it was apparently unsafe to\nfree them before calling cleanup.\n\nKnowing nothing about quirks in curl libraries, one thing that\nis mystery to me is that we slist_append() to other two lists\n(pragma_header and range_header) but we do not seem to ever free\nthem. Another slist dav_headers is allocated and then freed\ninside a function, so that call-pattern seems well-formed.\n\nNick, care to help us out?\n"},{"id":"20455","messageId":"1148313394.29228.11.camel@dv","threadId":"4220","inReplyTo":"87fyj4y1lx.fsf@mid.deneb.enyo.de","subject":"Re: Segfaults with USE_CURL_MULTI","fromName":"Pavel Roskin","fromEmail":"proski@gnu.org","sentAt":"2006-05-22T15:56:34Z","receivedAt":"2006-05-22T15:56:34Z","isPatch":false,"sender":{"key":"proski@gnu.org","avatar":null},"body":"On Sat, 2006-05-20 at 20:47 +0200, Florian Weimer wrote:\n> Is anybody else seeing segfaults on dumb HTTP pull with\n> USE_CURL_MULTI?\n\n_Everybody_ is seeing them! Just look for \"segfaults\" in the archives:\n\nhttp://marc.theaimsgroup.com/?l=git&w=2&r=1&s=segfaults&q=b\n\nThis patch looks promising, but I'm yet to test it:\nhttp://marc.theaimsgroup.com/?l=git&m=114816558325617&w=2\n\n> For example, this crashes for me:\n> \n> $ git clone http://git.enyo.de/fw/debian/debfoster.git upstream\n> \n> GDB shows that this happens inside the call to curl_multi_perform.\n\nSame for me:\nhttp://marc.theaimsgroup.com/?l=git&m=114790994726570&w=2\n\n-- \nRegards,\nPavel Roskin\n"},{"id":"21011","messageId":"20060531160731.GA12261@reactrix.com","threadId":"4220","inReplyTo":"7vverzzukg.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Remove possible segfault in http-fetch.","fromName":"Nick Hengeveld","fromEmail":"nickh@reactrix.com","sentAt":"2006-05-31T16:07:31Z","receivedAt":"2006-05-31T16:07:31Z","isPatch":true,"sender":{"key":"nickh@reactrix.com","avatar":null},"body":"On Sun, May 21, 2006 at 12:49:19AM -0700, Junio C Hamano wrote:\n\n> curl_easy_cleanup() which is called from http_cleanup() says it\n> is safe to remove the strings _after_ you call that function, so\n> I think the change makes sense -- it was apparently unsafe to\n> free them before calling cleanup.\n> \n> Knowing nothing about quirks in curl libraries, one thing that\n> is mystery to me is that we slist_append() to other two lists\n> (pragma_header and range_header) but we do not seem to ever free\n> them. Another slist dav_headers is allocated and then freed\n> inside a function, so that call-pattern seems well-formed.\n> \n> Nick, care to help us out?\n\nI just got back from a trip to the midwest and am still getting caught\nup. I was only gone for 10 days, you've all been quite busy...\n\nYou're correct wrt the other slists, I'll get to work on a patch for\nthat after I've caught up.\n\nI'm also doing additional testing to see whether this fixes the DAV/405\nsegfault as I think there may be something else going on there.\n\n-- \nFor a successful technology, reality must take precedence over public\nrelations, for nature cannot be fooled.\n"}]}