On Tue, 10 Feb 2009, Junio C Hamano wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > > > "Shawn O. Pearce" <spearce@xxxxxxxxxxx> writes: > > > >> Daniel Barkalow <barkalow@xxxxxxxxxxxx> wrote: > >>> On Tue, 10 Feb 2009, Shawn O. Pearce wrote: > >>> > > >>> > We should dump the cached_objects table in sha1_file.c during > >>> > a checkpoint in fast-import. > >>> > >>> No, that one's keyed by sha1, and doesn't get collisions; it's the > >>> delta_base_cache that's the issue; it's keyed by struct packed_git * and > >>> offset. > >> > >> Uh, yea, I realize that after I sent the message. Does this patch > >> fix it for you? > >> > >> --8<-- > >> Clear the delta base cache during fast-import checkpoint > >> > >> Otherwise we may reuse the same memory address for a totally > >> different "struct packed_git", and a previously cached object from > >> the prior occupant might be returned when trying to unpack an object > >> from the new pack. > > > > Can this be made more automatic? > > > > For example if you do this every time a new pack is installed to > > sha1_file(), like in add_packed_git() perhaps, wouldn't that be much less > > error prone? > > On second thought, I think fast-import is the only program that plays > funny games of feeding a packed_git that is *not* part of the real list of > packed_git installed in the system to unpack_entry(), so probably your > patch is a better idea. That's not the problem; the problem is calling free on a struct packed_git that has been given to unpack_entry(), because it raises the possibility of having the memory allocated to a different pack in the future and ending up with actively wrong entries in the delta cache, since it keys off of the pointer. I think free_pack_by_name() also needs to drop the entries that are from the freed pack, to avoid having repack able to get the same problem, although I wouldn't be surprised if repack happened to never allocate a new pack after freeing an old pack with stale delta cache entries, or never used the delta cache after that, simply because it does one thing and then exits. -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
