On Sun, 2 Mar 2008, Mike Hommey wrote: > On Sun, Mar 02, 2008 at 08:23:55PM +0100, Mike Hommey wrote: > > On Sun, Mar 02, 2008 at 07:08:57PM +0000, Gerrit Pape wrote: > > > Hi, as reported through http://bugs.debian.org/468836, I can reproduce > > > with current maint branch on Debian/unstable, but I don't know whether > > > it's a problem in curl, or in git. Maybe anyone with some experience > > > in curl can help on this? > > > > > > gdb gives this > > (...) > > > > valgrind gives better insight: > > ==862== Invalid read of size 4 > > ==862== at 0x493B32: fill_active_slots (http.c:441) > > ==862== by 0x493CF9: step_active_slots (http.c:459) > > ==862== by 0x493D6E: run_active_slot (http.c:479) > > ==862== by 0x493F8B: http_cleanup (http.c:296) > > ==862== by 0x494CA8: cleanup (http-walker.c:900) > > ==862== by 0x4911D6: walker_free (walker.c:315) > > ==862== by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81) > > ==862== by 0x404247: handle_internal_command (git.c:248) > > ==862== by 0x4049D4: main (git.c:412) > > ==862== Address 0x7c7c558 is 16 bytes inside a block of size 72 free'd > > ==862== at 0x4C20B2E: free (vg_replace_malloc.c:323) > > ==862== by 0x493F47: http_cleanup (http.c:301) > > ==862== by 0x494CA8: cleanup (http-walker.c:900) > > ==862== by 0x4911D6: walker_free (walker.c:315) > > ==862== by 0x44A149: cmd_http_fetch (builtin-http-fetch.c:81) > > ==862== by 0x404247: handle_internal_command (git.c:248) > > ==862== by 0x4049D4: main (git.c:412) > (...) > > > > It seems there is something wrong going on with slots... > > And the problem lies in the fact we run_active_slot() during cleanup, > which can end up going through all the slots starting at > active_queue_head, while we have freed the first slots... > > Now, why do we need to run slots when cleaning up ? AFAICT, it's always been that way. I assume there was code that set up all of the remaining transfers and then just called http_cleanup, relying on the callbacks to handle the receipt of the remaining data, but I'm not sure if that's still the case. On the other hand, I think that code is supposed to remove slots from the active queue as they get processed, so that run_active_slot() is always safe to call and just won't do anything if it's not needed in cleanup. So I'm guessing that we have list corruption due to code getting careless in error cases, in addition to cleanup code that possibly cares too much about finishing everything it can. (I don't really know the http.c code all that well, BTW; I've only interacted with it peripherally in reorganizing http-fetch into http-walker last summer) -Daniel *This .sig left intentionally blank* -- 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
