{"thread":{"id":"12433","subject":"git-http-fetch segfault, curl 7.18.0","startedAt":"2008-03-02T19:08:57Z","lastAt":"2008-03-03T19:30:16Z","messageCount":8,"participants":["Gerrit Pape","Mike Hommey","Daniel Barkalow"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"70685","messageId":"20080302190857.11027.qmail@28c5bbafb32cb2.315fe32.mid.smarden.org","threadId":"12433","inReplyTo":null,"subject":"git-http-fetch segfault, curl 7.18.0","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2008-03-02T19:08:57Z","receivedAt":"2008-03-02T19:08:57Z","isPatch":false,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"Hi, as reported through http://bugs.debian.org/468836, I can reproduce\nwith current maint branch on Debian/unstable, but I don't know whether\nit's a problem in curl, or in git.  Maybe anyone with some experience\nin curl can help on this?\n\n $ make\n [...]\n $ mkdir repo && cd repo && git init\n Initialized empty Git repository in .git/\n $ ../git-http-fetch -v -a -w remotes/origin/master ef227a3da916f53f5a9ef4266c27c283c1ab607b http://git.debian.org/git/users/eddyp-guest/test-segfault.git/\n got ef227a3da916f53f5a9ef4266c27c283c1ab607b\n walk ef227a3da916f53f5a9ef4266c27c283c1ab607b\n got 12d086d13b3e69b26389282fe031b1a15153d84c\n Getting alternates list for\n http://git.debian.org/git/users/eddyp-guest/test-segfault.git\n Getting pack list for\n http://git.debian.org/git/users/eddyp-guest/test-segfault.git\n got e25c7e03ea0c0d4d9bea5552ee629a25196ff43f\n got a8d762c979ac21da092732eafb6ded8751c3f63d\n Getting index for pack 06eec8e3e8f770e471d5f3f19c3b8ffa9159fd97\n got 31cdb53945ddfa3b7bc15e12fdf900024eec1a56\n Getting pack 06eec8e3e8f770e471d5f3f19c3b8ffa9159fd97\n  which contains 64580202061d4d8f449e09107ac6e60706142202\n walk 64580202061d4d8f449e09107ac6e60706142202\n error: Unable to find 30e8500073c101f656311906f5a00f0ccc0e8546 under http://git.debian.org/git/users/eddyp-guest/test-segfault.git\n Cannot obtain needed commit 30e8500073c101f656311906f5a00f0ccc0e8546\n while processing commit 64580202061d4d8f449e09107ac6e60706142202.\n $ echo $?\n 255\n $ ../git-http-fetch -v -a -w remotes/origin/master ef227a3da916f53f5a9ef4266c27c283c1ab607b http://git.debian.org/git/users/eddyp-guest/test-segfault.git/\n walk ef227a3da916f53f5a9ef4266c27c283c1ab607b\n walk 64580202061d4d8f449e09107ac6e60706142202\n Getting alternates list for\n http://git.debian.org/git/users/eddyp-guest/test-segfault.git\n Getting pack list for\n http://git.debian.org/git/users/eddyp-guest/test-segfault.git\n error: Unable to find 30e8500073c101f656311906f5a00f0ccc0e8546 under\n http://git.debian.org/git/users/eddyp-guest/test-segfault.git\n Cannot obtain needed commit 30e8500073c101f656311906f5a00f0ccc0e8546\n while processing commit 64580202061d4d8f449e09107ac6e60706142202.\n Waiting for\n http://git.debian.org/git/users/eddyp-guest/test-segfault.git/objects/8b/d9d5f451cb08435cdcb773dc8cdddf9f61d553\n Waiting for\n http://git.debian.org/git/users/eddyp-guest/test-segfault.git/objects/ef/b3c1b344b887ffdb5d917eca2a28e5f176613c\n Segmentation fault\n\ngdb gives this\n\n (gdb) r -v -a -w remotes/origin/master ef227a3da916f53f5a9ef4266c27c283c1ab607b http://git.debian.org/git/users/eddyp-guest/test-segfault.git/\n Starting program: /usr/local/src/Debian/GIT/git/git-http-fetch -v -a -w\n remotes/origin/master  ef227a3da916f53f5a9ef4266c27c283c1ab607b\n http://git.debian.org/git/users/eddyp-guest/test-segfault.git/\n [Thread debugging using libthread_db enabled]\n walk ef227a3da916f53f5a9ef4266c27c283c1ab607b\n [New Thread 0x3002c220 (LWP 19305)]\n walk 64580202061d4d8f449e09107ac6e60706142202\n Getting alternates list for http://git.debian.org/git/users/eddyp-guest/test-segfault.git\n Getting pack list for http://git.debian.org/git/users/eddyp-guest/test-segfault.git\n error: Unable to find 30e8500073c101f656311906f5a00f0ccc0e8546 under http://git.debian.org/git/users/eddyp-guest/test-segfault.git\n Cannot obtain needed commit 30e8500073c101f656311906f5a00f0ccc0e8546\n while processing commit 64580202061d4d8f449e09107ac6e60706142202.\n Waiting for http://git.debian.org/git/users/eddyp-guest/test-segfault.git/objects/8b/d9d5f451cb08435cdcb773dc8cdddf9f61d553\n Waiting for http://git.debian.org/git/users/eddyp-guest/test-segfault.git/objects/3a/36bcfe419fe97021a330516c25e8c8d6a61b23\n Waiting for http://git.debian.org/git/users/eddyp-guest/test-segfault.git/objects/f9/1f9480407a3786f0a66855af7c64da9360c214\n\n Program received signal SIGSEGV, Segmentation fault.\n [Switching to Thread 0x3002c220 (LWP 19305)]\n 0x0ffbaddc in ?? () from /usr/lib/libcurl-gnutls.so.4\n (gdb) bt\n #0  0x0ffbaddc in ?? () from /usr/lib/libcurl-gnutls.so.4\n #1  0x0ffbadc4 in ?? () from /usr/lib/libcurl-gnutls.so.4\n #2  0x0ffc3f0c in curl_easy_cleanup () from /usr/lib/libcurl-gnutls.so.4\n #3  0x100a4e8c in fill_active_slots () at http.c:442\n #4  0x100a5100 in step_active_slots () at http.c:459\n #5  0x100a519c in run_active_slot (slot=0x10169f08) at http.c:479\n #6  0x100a5478 in http_cleanup () at http.c:296\n #7  0x100a6514 in cleanup (walker=<value optimized out>) at http-walker.c:900\n #8  0x100a2550 in walker_free (walker=<value optimized out>) at walker.c:315\n #9  0x1004df68 in cmd_http_fetch (argc=<value optimized out>, argv=0x7fbfe784, \n     prefix=<value optimized out>) at builtin-http-fetch.c:81\n #10 0x100045e8 in handle_internal_command (argc=7, argv=0x7fbfe784) at git.c:259\n #11 0x10004f88 in main (argc=7, argv=0x7fbfe784) at git.c:420\n (gdb) \n\nand sometimes\n\n (gdb) r -v -a -w remotes/origin/master ef227a3da916f53f5a9ef4266c27c283c1ab607b http://git.debian.org/git/users/eddyp-guest/test-segfault.git/\n Starting program: /usr/local/src/Debian/GIT/git/git-http-fetch -v -a -w remotes/origin/master  ef227a3da916f53f5a9ef4266c27c283c1ab607b http://git.debian.org/git/users/eddyp-guest/test-segfault.git/\n [Thread debugging using libthread_db enabled]\n walk ef227a3da916f53f5a9ef4266c27c283c1ab607b\n [New Thread 0x3002c220 (LWP 20097)]\n walk 64580202061d4d8f449e09107ac6e60706142202\n Getting alternates list for http://git.debian.org/git/users/eddyp-guest/test-segfault.git\n Getting pack list for http://git.debian.org/git/users/eddyp-guest/test-segfault.git\n error: Unable to find 30e8500073c101f656311906f5a00f0ccc0e8546 under http://git.debian.org/git/users/eddyp-guest/test-segfault.git\n Cannot obtain needed commit 30e8500073c101f656311906f5a00f0ccc0e8546\n while processing commit 64580202061d4d8f449e09107ac6e60706142202.\n Waiting for http://git.debian.org/git/users/eddyp-guest/test-segfault.git/objects/61/6e4145cf75c474bafa94f1dd917ed0a6f421fc\n Waiting for http://git.debian.org/git/users/eddyp-guest/test-segfault.git/objects/ef/b3c1b344b887ffdb5d917eca2a28e5f176613c\n\n Program received signal SIGSEGV, Segmentation fault.\n [Switching to Thread 0x3002c220 (LWP 20097)]\n fill_active_slots () at http.c:441\n 441                     if (!slot->in_use && slot->curl != NULL) {\n (gdb) \n\nThanks, Gerrit.\n"},{"id":"70686","messageId":"20080302192355.GA625@glandium.org","threadId":"12433","inReplyTo":"20080302190857.11027.qmail@28c5bbafb32cb2.315fe32.mid.smarden.org","subject":"Re: git-http-fetch segfault, curl 7.18.0","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-03-02T19:23:55Z","receivedAt":"2008-03-02T19:23:55Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sun, Mar 02, 2008 at 07:08:57PM +0000, Gerrit Pape wrote:\n> Hi, as reported through http://bugs.debian.org/468836, I can reproduce\n> with current maint branch on Debian/unstable, but I don't know whether\n> it's a problem in curl, or in git.  Maybe anyone with some experience\n> in curl can help on this?\n> \n> gdb gives this\n(...)\n\nvalgrind gives better insight:\n==862== Invalid read of size 4\n==862==    at 0x493B32: fill_active_slots (http.c:441)\n==862==    by 0x493CF9: step_active_slots (http.c:459)\n==862==    by 0x493D6E: run_active_slot (http.c:479)\n==862==    by 0x493F8B: http_cleanup (http.c:296)\n==862==    by 0x494CA8: cleanup (http-walker.c:900)\n==862==    by 0x4911D6: walker_free (walker.c:315)\n==862==    by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81)\n==862==    by 0x404247: handle_internal_command (git.c:248)\n==862==    by 0x4049D4: main (git.c:412)\n==862==  Address 0x7c7c558 is 16 bytes inside a block of size 72 free'd\n==862==    at 0x4C20B2E: free (vg_replace_malloc.c:323)\n==862==    by 0x493F47: http_cleanup (http.c:301)\n==862==    by 0x494CA8: cleanup (http-walker.c:900)\n==862==    by 0x4911D6: walker_free (walker.c:315)\n==862==    by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81)\n==862==    by 0x404247: handle_internal_command (git.c:248)\n==862==    by 0x4049D4: main (git.c:412)\n==862== \n==862== Invalid read of size 8\n==862==    at 0x493B50: fill_active_slots (http.c:445)\n==862==    by 0x493CF9: step_active_slots (http.c:459)\n==862==    by 0x493D6E: run_active_slot (http.c:479)\n==862==    by 0x493F8B: http_cleanup (http.c:296)\n==862==    by 0x494CA8: cleanup (http-walker.c:900)\n==862==    by 0x4911D6: walker_free (walker.c:315)\n==862==    by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81)\n==862==    by 0x404247: handle_internal_command (git.c:248)\n==862==    by 0x4049D4: main (git.c:412)\n==862==  Address 0x7c7c588 is 64 bytes inside a block of size 72 free'd\n==862==    at 0x4C20B2E: free (vg_replace_malloc.c:323)\n==862==    by 0x493F47: http_cleanup (http.c:301)\n==862==    by 0x494CA8: cleanup (http-walker.c:900)\n==862==    by 0x4911D6: walker_free (walker.c:315)\n==862==    by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81)\n==862==    by 0x404247: handle_internal_command (git.c:248)\n==862==    by 0x4049D4: main (git.c:412)\n==862== \n==862== Invalid read of size 8\n==862==    at 0x493BFD: process_curl_messages (http.c:74)\n==862==    by 0x493CF4: step_active_slots (http.c:458)\n==862==    by 0x493D6E: run_active_slot (http.c:479)\n==862==    by 0x493F8B: http_cleanup (http.c:296)\n==862==    by 0x494CA8: cleanup (http-walker.c:900)\n==862==    by 0x4911D6: walker_free (walker.c:315)\n==862==    by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81)\n==862==    by 0x404247: handle_internal_command (git.c:248)\n==862==    by 0x4049D4: main (git.c:412)\n==862==  Address 0x7c7c548 is 0 bytes inside a block of size 72 free'd\n==862==    at 0x4C20B2E: free (vg_replace_malloc.c:323)\n==862==    by 0x493F47: http_cleanup (http.c:301)\n==862==    by 0x494CA8: cleanup (http-walker.c:900)\n==862==    by 0x4911D6: walker_free (walker.c:315)\n==862==    by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81)\n==862==    by 0x404247: handle_internal_command (git.c:248)\n==862==    by 0x4049D4: main (git.c:412)\n==862== \n==862== Invalid read of size 8\n==862==    at 0x493BF0: process_curl_messages (http.c:76)\n==862==    by 0x493CF4: step_active_slots (http.c:458)\n==862==    by 0x493D6E: run_active_slot (http.c:479)\n==862==    by 0x493F8B: http_cleanup (http.c:296)\n==862==    by 0x494CA8: cleanup (http-walker.c:900)\n==862==    by 0x4911D6: walker_free (walker.c:315)\n==862==    by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81)\n==862==    by 0x404247: handle_internal_command (git.c:248)\n==862==    by 0x4049D4: main (git.c:412)\n==862==  Address 0x7c7c588 is 64 bytes inside a block of size 72 free'd\n==862==    at 0x4C20B2E: free (vg_replace_malloc.c:323)\n==862==    by 0x493F47: http_cleanup (http.c:301)\n==862==    by 0x494CA8: cleanup (http-walker.c:900)\n==862==    by 0x4911D6: walker_free (walker.c:315)\n==862==    by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81)\n==862==    by 0x404247: handle_internal_command (git.c:248)\n==862==    by 0x4049D4: main (git.c:412)\n==862== \n==862== Invalid read of size 4\n==862==    at 0x494372: get_active_slot (http.c:336)\n==862==    by 0x49504A: start_object_request (http-walker.c:195)\n==862==    by 0x495413: fill_active_slot (http-walker.c:321)\n==862==    by 0x493B1C: fill_active_slots (http.c:433)\n==862==    by 0x493CF9: step_active_slots (http.c:459)\n==862==    by 0x493D6E: run_active_slot (http.c:479)\n==862==    by 0x493F8B: http_cleanup (http.c:296)\n==862==    by 0x494CA8: cleanup (http-walker.c:900)\n==862==    by 0x4911D6: walker_free (walker.c:315)\n==862==    by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81)\n==862==    by 0x404247: handle_internal_command (git.c:248)\n==862==    by 0x4049D4: main (git.c:412)\n==862==  Address 0x7c7c558 is 16 bytes inside a block of size 72 free'd\n==862==    at 0x4C20B2E: free (vg_replace_malloc.c:323)\n==862==    by 0x493F47: http_cleanup (http.c:301)\n==862==    by 0x494CA8: cleanup (http-walker.c:900)\n==862==    by 0x4911D6: walker_free (walker.c:315)\n==862==    by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81)\n==862==    by 0x404247: handle_internal_command (git.c:248)\n==862==    by 0x4049D4: main (git.c:412)\n==862== \n==862== Invalid read of size 8\n==862==    at 0x494379: get_active_slot (http.c:337)\n==862==    by 0x49504A: start_object_request (http-walker.c:195)\n==862==    by 0x495413: fill_active_slot (http-walker.c:321)\n==862==    by 0x493B1C: fill_active_slots (http.c:433)\n==862==    by 0x493CF9: step_active_slots (http.c:459)\n==862==    by 0x493D6E: run_active_slot (http.c:479)\n==862==    by 0x493F8B: http_cleanup (http.c:296)\n==862==    by 0x494CA8: cleanup (http-walker.c:900)\n==862==    by 0x4911D6: walker_free (walker.c:315)\n==862==    by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81)\n==862==    by 0x404247: handle_internal_command (git.c:248)\n==862==    by 0x4049D4: main (git.c:412)\n==862==  Address 0x7c7c588 is 64 bytes inside a block of size 72 free'd\n==862==    at 0x4C20B2E: free (vg_replace_malloc.c:323)\n==862==    by 0x493F47: http_cleanup (http.c:301)\n==862==    by 0x494CA8: cleanup (http-walker.c:900)\n==862==    by 0x4911D6: walker_free (walker.c:315)\n==862==    by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81)\n==862==    by 0x404247: handle_internal_command (git.c:248)\n==862==    by 0x4049D4: main (git.c:412)\n\nIt seems there is something wrong going on with slots...\n\nMike\n"},{"id":"70689","messageId":"20080302200309.GA2070@glandium.org","threadId":"12433","inReplyTo":"20080302192355.GA625@glandium.org","subject":"Re: git-http-fetch segfault, curl 7.18.0","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-03-02T20:03:09Z","receivedAt":"2008-03-02T20:03:09Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sun, Mar 02, 2008 at 08:23:55PM +0100, Mike Hommey wrote:\n> On Sun, Mar 02, 2008 at 07:08:57PM +0000, Gerrit Pape wrote:\n> > Hi, as reported through http://bugs.debian.org/468836, I can reproduce\n> > with current maint branch on Debian/unstable, but I don't know whether\n> > it's a problem in curl, or in git.  Maybe anyone with some experience\n> > in curl can help on this?\n> > \n> > gdb gives this\n> (...)\n> \n> valgrind gives better insight:\n> ==862== Invalid read of size 4\n> ==862==    at 0x493B32: fill_active_slots (http.c:441)\n> ==862==    by 0x493CF9: step_active_slots (http.c:459)\n> ==862==    by 0x493D6E: run_active_slot (http.c:479)\n> ==862==    by 0x493F8B: http_cleanup (http.c:296)\n> ==862==    by 0x494CA8: cleanup (http-walker.c:900)\n> ==862==    by 0x4911D6: walker_free (walker.c:315)\n> ==862==    by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81)\n> ==862==    by 0x404247: handle_internal_command (git.c:248)\n> ==862==    by 0x4049D4: main (git.c:412)\n> ==862==  Address 0x7c7c558 is 16 bytes inside a block of size 72 free'd\n> ==862==    at 0x4C20B2E: free (vg_replace_malloc.c:323)\n> ==862==    by 0x493F47: http_cleanup (http.c:301)\n> ==862==    by 0x494CA8: cleanup (http-walker.c:900)\n> ==862==    by 0x4911D6: walker_free (walker.c:315)\n> ==862==    by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81)\n> ==862==    by 0x404247: handle_internal_command (git.c:248)\n> ==862==    by 0x4049D4: main (git.c:412)\n(...)\n> \n> It seems there is something wrong going on with slots...\n\nAnd the problem lies in the fact we run_active_slot() during cleanup,\nwhich can end up going through all the slots starting at\nactive_queue_head, while we have freed the first slots...\n\nNow, why do we need to run slots when cleaning up ?\n\nMike\n"},{"id":"70690","messageId":"alpine.LNX.1.00.0803021508340.19665@iabervon.org","threadId":"12433","inReplyTo":"20080302200309.GA2070@glandium.org","subject":"Re: git-http-fetch segfault, curl 7.18.0","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-03-02T20:20:17Z","receivedAt":"2008-03-02T20:20:17Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Sun, 2 Mar 2008, Mike Hommey wrote:\n\n> On Sun, Mar 02, 2008 at 08:23:55PM +0100, Mike Hommey wrote:\n> > On Sun, Mar 02, 2008 at 07:08:57PM +0000, Gerrit Pape wrote:\n> > > Hi, as reported through http://bugs.debian.org/468836, I can reproduce\n> > > with current maint branch on Debian/unstable, but I don't know whether\n> > > it's a problem in curl, or in git.  Maybe anyone with some experience\n> > > in curl can help on this?\n> > > \n> > > gdb gives this\n> > (...)\n> > \n> > valgrind gives better insight:\n> > ==862== Invalid read of size 4\n> > ==862==    at 0x493B32: fill_active_slots (http.c:441)\n> > ==862==    by 0x493CF9: step_active_slots (http.c:459)\n> > ==862==    by 0x493D6E: run_active_slot (http.c:479)\n> > ==862==    by 0x493F8B: http_cleanup (http.c:296)\n> > ==862==    by 0x494CA8: cleanup (http-walker.c:900)\n> > ==862==    by 0x4911D6: walker_free (walker.c:315)\n> > ==862==    by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81)\n> > ==862==    by 0x404247: handle_internal_command (git.c:248)\n> > ==862==    by 0x4049D4: main (git.c:412)\n> > ==862==  Address 0x7c7c558 is 16 bytes inside a block of size 72 free'd\n> > ==862==    at 0x4C20B2E: free (vg_replace_malloc.c:323)\n> > ==862==    by 0x493F47: http_cleanup (http.c:301)\n> > ==862==    by 0x494CA8: cleanup (http-walker.c:900)\n> > ==862==    by 0x4911D6: walker_free (walker.c:315)\n> > ==862==    by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81)\n> > ==862==    by 0x404247: handle_internal_command (git.c:248)\n> > ==862==    by 0x4049D4: main (git.c:412)\n> (...)\n> > \n> > It seems there is something wrong going on with slots...\n> \n> And the problem lies in the fact we run_active_slot() during cleanup,\n> which can end up going through all the slots starting at\n> active_queue_head, while we have freed the first slots...\n> \n> Now, why do we need to run slots when cleaning up ?\n\nAFAICT, it's always been that way. I assume there was code that set up all \nof the remaining transfers and then just called http_cleanup, relying on \nthe callbacks to handle the receipt of the remaining data, but I'm not \nsure if that's still the case. On the other hand, I think that code is \nsupposed to remove slots from the active queue as they get processed, so \nthat run_active_slot() is always safe to call and just won't do anything \nif it's not needed in cleanup.\n\nSo I'm guessing that we have list corruption due to code getting careless \nin error cases, in addition to cleanup code that possibly cares too much \nabout finishing everything it can.\n\n(I don't really know the http.c code all that well, BTW; I've only \ninteracted with it peripherally in reorganizing http-fetch into \nhttp-walker last summer)\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"70691","messageId":"1204489713-8696-1-git-send-email-mh@glandium.org","threadId":"12433","inReplyTo":"20080302200309.GA2070@glandium.org","subject":"[PATCH] Fix random crashes in http_cleanup()","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-03-02T20:28:33Z","receivedAt":"2008-03-02T20:28:33Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"For some reason, http_cleanup was running all active slots, which could\nlead in situations where a freed slot would be accessed in\nfill_active_slots. OTOH, we are cleaning up, which means the caller\ndoesn't care about pending requests. Just forget about them instead\nor running them.\n\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n > Now, why do we need to run slots when cleaning up ?\n\n I just think we don't care, so this fix should be okay.\n\n http.c |   11 +++--------\n 1 files changed, 3 insertions(+), 8 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 5925d07..9683e46 100644\n--- a/http.c\n+++ b/http.c\n@@ -287,17 +287,12 @@ void http_cleanup(void)\n \n \twhile (slot != NULL) {\n \t\tstruct active_request_slot *next = slot->next;\n+\t\tif (slot->curl != NULL) {\n #ifdef USE_CURL_MULTI\n-\t\tif (slot->in_use) {\n-\t\t\tcurl_easy_getinfo(slot->curl,\n-\t\t\t\t\t  CURLINFO_EFFECTIVE_URL,\n-\t\t\t\t\t  &wait_url);\n-\t\t\tfprintf(stderr, \"Waiting for %s\\n\", wait_url);\n-\t\t\trun_active_slot(slot);\n-\t\t}\n+\t\t\tcurl_multi_remove_handle(curlm, slot->curl);\n #endif\n-\t\tif (slot->curl != NULL)\n \t\t\tcurl_easy_cleanup(slot->curl);\n+\t\t}\n \t\tfree(slot);\n \t\tslot = next;\n \t}\n-- \n1.5.4.3.368.g272aa.dirty\n\n"},{"id":"70692","messageId":"20080302203344.GA8760@glandium.org","threadId":"12433","inReplyTo":"alpine.LNX.1.00.0803021508340.19665@iabervon.org","subject":"Re: git-http-fetch segfault, curl 7.18.0","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-03-02T20:33:44Z","receivedAt":"2008-03-02T20:33:44Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sun, Mar 02, 2008 at 03:20:17PM -0500, Daniel Barkalow wrote:\n> > And the problem lies in the fact we run_active_slot() during cleanup,\n> > which can end up going through all the slots starting at\n> > active_queue_head, while we have freed the first slots...\n> > \n> > Now, why do we need to run slots when cleaning up ?\n> \n> AFAICT, it's always been that way. I assume there was code that set up all \n> of the remaining transfers and then just called http_cleanup, relying on \n> the callbacks to handle the receipt of the remaining data, but I'm not \n> sure if that's still the case.\n\nIt doesn't look like it is stil the case.\n\n> On the other hand, I think that code is \n> supposed to remove slots from the active queue as they get processed, so \n> that run_active_slot() is always safe to call and just won't do anything \n> if it's not needed in cleanup.\n> \n> So I'm guessing that we have list corruption due to code getting careless \n> in error cases, in addition to cleanup code that possibly cares too much \n> about finishing everything it can.\n\nThat's in fill_active_slots that it's trying to go through all slots\nstarting at active_queue_head, which is likely to be freed at this\npoint. The fix I sent earlier just throws all active slots, which should\njust be fine now.\n\nMike\n"},{"id":"70768","messageId":"20080303100141.15600.qmail@e81d07a86798f7.315fe32.mid.smarden.org","threadId":"12433","inReplyTo":"1204489713-8696-1-git-send-email-mh@glandium.org","subject":"Re: [PATCH] Fix random crashes in http_cleanup()","fromName":"Gerrit Pape","fromEmail":"pape@smarden.org","sentAt":"2008-03-03T10:01:41Z","receivedAt":"2008-03-03T10:01:41Z","isPatch":true,"sender":{"key":"pape@smarden.org","avatar":"https://avatars.githubusercontent.com/u/143170252?v=4"},"body":"On Sun, Mar 02, 2008 at 09:28:33PM +0100, Mike Hommey wrote:\n> For some reason, http_cleanup was running all active slots, which could\n> lead in situations where a freed slot would be accessed in\n> fill_active_slots. OTOH, we are cleaning up, which means the caller\n> doesn't care about pending requests. Just forget about them instead\n> or running them.\n\nHi, I can confirm that this fixes the segfault we managed to reproduce,\nbut didn't look deeper into the changes.\n\nThere's a warning when compiling http.c, trivial to fix:\n\n      CC http.o\n  http.c: In function ‘http_cleanup’:\n  http.c:288: warning: unused variable ‘wait_url’\n\nThanks!, Gerrit.\n"},{"id":"70832","messageId":"1204572616-25721-1-git-send-email-mh@glandium.org","threadId":"12433","inReplyTo":"20080303100141.15600.qmail@e81d07a86798f7.315fe32.mid.smarden.org","subject":"[PATCH v2] Fix random crashes in http_cleanup()","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2008-03-03T19:30:16Z","receivedAt":"2008-03-03T19:30:16Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"For some reason, http_cleanup was running all active slots, which could\nlead in situations where a freed slot would be accessed in\nfill_active_slots. OTOH, we are cleaning up, which means the caller\ndoesn't care about pending requests. Just forget about them instead\nor running them.\n\nSigned-off-by: Mike Hommey <mh@glandium.org>\n---\n http.c |   14 +++-----------\n 1 files changed, 3 insertions(+), 11 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 8e554c0..256a5f1 100644\n--- a/http.c\n+++ b/http.c\n@@ -284,23 +284,15 @@ void http_init(struct remote *remote)\n void http_cleanup(void)\n {\n \tstruct active_request_slot *slot = active_queue_head;\n-#ifdef USE_CURL_MULTI\n-\tchar *wait_url;\n-#endif\n \n \twhile (slot != NULL) {\n \t\tstruct active_request_slot *next = slot->next;\n+\t\tif (slot->curl != NULL) {\n #ifdef USE_CURL_MULTI\n-\t\tif (slot->in_use) {\n-\t\t\tcurl_easy_getinfo(slot->curl,\n-\t\t\t\t\t  CURLINFO_EFFECTIVE_URL,\n-\t\t\t\t\t  &wait_url);\n-\t\t\tfprintf(stderr, \"Waiting for %s\\n\", wait_url);\n-\t\t\trun_active_slot(slot);\n-\t\t}\n+\t\t\tcurl_multi_remove_handle(curlm, slot->curl);\n #endif\n-\t\tif (slot->curl != NULL)\n \t\t\tcurl_easy_cleanup(slot->curl);\n+\t\t}\n \t\tfree(slot);\n \t\tslot = next;\n \t}\n-- \n1.5.4.3.368.g272aa.dirty\n\n"}]}