{"thread":{"id":"17690","subject":"fact-import: failed to apply delta","startedAt":"2009-02-10T03:26:16Z","lastAt":"2009-02-11T18:33:21Z","messageCount":24,"participants":["Daniel Barkalow","Johannes Schindelin","Shawn O. Pearce","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"103945","messageId":"alpine.LNX.1.00.0902092218050.19665@iabervon.org","threadId":"17690","inReplyTo":null,"subject":"fact-import: failed to apply delta","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-02-10T03:26:16Z","receivedAt":"2009-02-10T03:26:16Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"I'm getting a \"fatal: failed to apply delta\" from fast-import. I'm using a \nlot of checkpoints, and I haven't had it happen without making progress, \nso I was eventually able to import what I was importing (bunch of stuff I \ncan't distribute, imported from perforce with the latest version of my p4 \nimporter that I'm still working on). Also, everything that was saved by \nthe checkpoints was valid and correct (at least after the fact).\n\nI'm going to see if it's reproducable, and, if so, if I can get a test \ncase that I can distribute, but I wanted to post to see if anyone had any \nspecial debugging advice for this error message and program combination.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"103965","messageId":"alpine.DEB.1.00.0902101126040.10279@pacific.mpi-cbg.de","threadId":"17690","inReplyTo":"alpine.LNX.1.00.0902092218050.19665@iabervon.org","subject":"Re: fact-import: failed to apply delta","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-10T10:28:30Z","receivedAt":"2009-02-10T10:28:30Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\n[nice typo in the subject BTW]\n\nOn Mon, 9 Feb 2009, Daniel Barkalow wrote:\n\n> I'm getting a \"fatal: failed to apply delta\" from fast-import. I'm using \n> a lot of checkpoints, and I haven't had it happen without making \n> progress, so I was eventually able to import what I was importing (bunch \n> of stuff I can't distribute, imported from perforce with the latest \n> version of my p4 importer that I'm still working on). Also, everything \n> that was saved by the checkpoints was valid and correct (at least after \n> the fact).\n> \n> I'm going to see if it's reproducable, and, if so, if I can get a test \n> case that I can distribute, but I wanted to post to see if anyone had \n> any special debugging advice for this error message and program \n> combination.\n\nI see three likely candidates: two in index-pack.c and one in sha1_file.c. \nMy advice: instrument the code (IOW litter the code with debug output that \ntells you where it did what), and then run it on the same test case you \nhad the problems.\n\nCiao,\nDscho\n"},{"id":"104027","messageId":"20090210155626.GM30949@spearce.org","threadId":"17690","inReplyTo":"alpine.DEB.1.00.0902101126040.10279@pacific.mpi-cbg.de","subject":"Re: fact-import: failed to apply delta","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-10T15:56:26Z","receivedAt":"2009-02-10T15:56:26Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Schindelin wrote:\n> On Mon, 9 Feb 2009, Daniel Barkalow wrote:\n> \n> > I'm getting a \"fatal: failed to apply delta\" from fast-import. I'm using \n> > a lot of checkpoints, and I haven't had it happen without making \n> > progress, so I was eventually able to import what I was importing (bunch \n> > of stuff I can't distribute, imported from perforce with the latest \n> > version of my p4 importer that I'm still working on). Also, everything \n> > that was saved by the checkpoints was valid and correct (at least after \n> > the fact).\n> > \n> > I'm going to see if it's reproducable, and, if so, if I can get a test \n> > case that I can distribute, but I wanted to post to see if anyone had \n> > any special debugging advice for this error message and program \n> > combination.\n> \n> I see three likely candidates: two in index-pack.c and one in sha1_file.c. \n\nIt has to be the one in sha1_file.c. fast-import never calls into the\ncode in index-pack.c.\n\n> My advice: instrument the code (IOW litter the code with debug output that \n> tells you where it did what), and then run it on the same test case you \n> had the problems.\n\nMy initial guess is, we're probably having trouble reading from\nthe pack we are writing. This shouldn't be possible; fast-import\nuses an index of all objects in memory to locate the byte offset\nand then reads from that block.\n\nI wonder if this is a write(2) vs. mmap(2) inconsistency in the\nVM system of your OS. fast-import plays some games here where\nit is actively writing into areas that might have been mmap'd.\nWe should be closing the existing mmap's and opening them again\nwhen we switch from writing to reading (see gfi_unpack_entry),\nbut if that's failing for some reason then we might be trying to\nread a VM page which isn't correctly initialized, and the delta\nisn't valid, and we can't inflate it.\n\n-- \nShawn.\n"},{"id":"104045","messageId":"alpine.LNX.1.00.0902101118110.19665@iabervon.org","threadId":"17690","inReplyTo":"20090210155626.GM30949@spearce.org","subject":"Re: fact-import: failed to apply delta","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-02-10T17:15:52Z","receivedAt":"2009-02-10T17:15:52Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 10 Feb 2009, Shawn O. Pearce wrote:\n\n> Johannes Schindelin wrote:\n> > On Mon, 9 Feb 2009, Daniel Barkalow wrote:\n> > \n> > > I'm getting a \"fatal: failed to apply delta\" from fast-import. I'm using \n> > > a lot of checkpoints, and I haven't had it happen without making \n> > > progress, so I was eventually able to import what I was importing (bunch \n> > > of stuff I can't distribute, imported from perforce with the latest \n> > > version of my p4 importer that I'm still working on). Also, everything \n> > > that was saved by the checkpoints was valid and correct (at least after \n> > > the fact).\n> > > \n> > > I'm going to see if it's reproducable, and, if so, if I can get a test \n> > > case that I can distribute, but I wanted to post to see if anyone had \n> > > any special debugging advice for this error message and program \n> > > combination.\n> > \n> > I see three likely candidates: two in index-pack.c and one in sha1_file.c. \n> \n> It has to be the one in sha1_file.c. fast-import never calls into the\n> code in index-pack.c.\n\nYup, it's that one. I added some sizes to the message and got them.\n\n> > My advice: instrument the code (IOW litter the code with debug output that \n> > tells you where it did what), and then run it on the same test case you \n> > had the problems.\n> \n> My initial guess is, we're probably having trouble reading from\n> the pack we are writing. This shouldn't be possible; fast-import\n> uses an index of all objects in memory to locate the byte offset\n> and then reads from that block.\n>\n> I wonder if this is a write(2) vs. mmap(2) inconsistency in the\n> VM system of your OS. \n\nThis is Linux 2.6.22 (ubuntu generic x86), so I don't think it does \nanything we don't expect.\n\n> fast-import plays some games here where it is actively writing into \n> areas that might have been mmap'd. We should be closing the existing \n> mmap's and opening them again when we switch from writing to reading \n> (see gfi_unpack_entry), but if that's failing for some reason then we \n> might be trying to read a VM page which isn't correctly initialized, and \n> the delta isn't valid, and we can't inflate it.\n\nI wonder if we're somehow getting the wrong object. I'm getting a \nbase_size of 584 but the object pointed to is a tree of size 1882. This \nseems to me like correctly initialized memory that just isn't what we \nwanted.\n\nIs there some way to see if the pack it was writing is actually corrupt \n(beyond not having the hash set)?\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"104047","messageId":"20090210172212.GR30949@spearce.org","threadId":"17690","inReplyTo":"alpine.LNX.1.00.0902101118110.19665@iabervon.org","subject":"Re: fact-import: failed to apply delta","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-10T17:22:12Z","receivedAt":"2009-02-10T17:22:12Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Daniel Barkalow wrote:\n> \n> Is there some way to see if the pack it was writing is actually corrupt \n> (beyond not having the hash set)?\n\nHmm. If you have the pack fragment, its going to take some editing\nto get it through the existing validation tools.\n\nFirst you need to know how many objects are in the pack just so you\ncan update the object count, which is a 4 byte network byte order\nfield starting at offset 8. Then you need the SHA-1 checksum of\nthe entire pack appended onto the end, as the last 20 bytes.\n\nOnce the pack is \"closed\" (by applying those fixes), you can run it\nthrough both index-pack and verify-pack. They pick up most errors,\nespecially a delta apply sort of error. They won't find corrupt\ntree modes though, or object connectivity errors.\n\n-- \nShawn.\n"},{"id":"104051","messageId":"alpine.LNX.1.00.0902101226580.19665@iabervon.org","threadId":"17690","inReplyTo":"20090210172212.GR30949@spearce.org","subject":"Re: fact-import: failed to apply delta","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-02-10T17:47:40Z","receivedAt":"2009-02-10T17:47:40Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 10 Feb 2009, Shawn O. Pearce wrote:\n\n> Daniel Barkalow wrote:\n> > \n> > Is there some way to see if the pack it was writing is actually corrupt \n> > (beyond not having the hash set)?\n> \n> Hmm. If you have the pack fragment, its going to take some editing\n> to get it through the existing validation tools.\n\nActually, I went for the other end; I made close_pack_windows() not mind \nthe open windows (hey, it's dying anyway in my case, nobody's going to \nwrite more), and the results passed verification and \"git fsck --full\" \nwith just a few dangling blobs and a dangling commit. So it seems to me \nthat it has to be wrong information in memory.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"104063","messageId":"20090210191220.GT30949@spearce.org","threadId":"17690","inReplyTo":"alpine.LNX.1.00.0902101226580.19665@iabervon.org","subject":"Re: fact-import: failed to apply delta","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-10T19:12:20Z","receivedAt":"2009-02-10T19:12:20Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Daniel Barkalow wrote:\n> \n> Actually, I went for the other end; I made close_pack_windows() not mind \n> the open windows (hey, it's dying anyway in my case, nobody's going to \n> write more), and the results passed verification and \"git fsck --full\" \n> with just a few dangling blobs and a dangling commit. So it seems to me \n> that it has to be wrong information in memory.\n\nLike the wrong offset within the pack for the object start?\n\nCan you compare the offsets you are getting during\nunpack_delta_entry() against what verify-pack -v\nshows for the same file? They should agree, unless\nwe're somehow wrong in memory within fast-import.\n\nBut then, the output pack-*.idx file created when\nfast-import closed the pack would be wrong too.\n\n-- \nShawn.\n"},{"id":"104066","messageId":"alpine.LNX.1.00.0902101427300.19665@iabervon.org","threadId":"17690","inReplyTo":"20090210191220.GT30949@spearce.org","subject":"Re: fact-import: failed to apply delta","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-02-10T20:03:32Z","receivedAt":"2009-02-10T20:03:32Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 10 Feb 2009, Shawn O. Pearce wrote:\n\n> Daniel Barkalow wrote:\n> > \n> > Actually, I went for the other end; I made close_pack_windows() not mind \n> > the open windows (hey, it's dying anyway in my case, nobody's going to \n> > write more), and the results passed verification and \"git fsck --full\" \n> > with just a few dangling blobs and a dangling commit. So it seems to me \n> > that it has to be wrong information in memory.\n> \n> Like the wrong offset within the pack for the object start?\n> \n> Can you compare the offsets you are getting during\n> unpack_delta_entry() against what verify-pack -v\n> shows for the same file? They should agree, unless\n> we're somehow wrong in memory within fast-import.\n\nIs there some easy way to tell what object it was having problems with \nwhen it failed to unpack? I've got a whole lot of objects.\n\nOn the other hand, there's something interesting:\n\nThe expected size of the base is 1882, while the actual size is 151. The \nbase offset it found was 12.\n\nI'm using \"checkpoint\" a lot, so I've got 24 packs. Two of them have tree \nobjects of size 1882 at offset 12; a different one has a tree object of \nsize 151 at offset 12. The one with the object of size 151 was the one \nthat was still open at the end. There's no tree of size 1882 in this pack, \nnor in any other pack that has a tree of size 151.\n\nSo maybe it's right about the offsets and all, but it's confused about \nwhich pack something was in? Maybe it cached something when the pack \ncontaining the object it wants was open, and it ended up thinking it was \nin the pack that's now open rather than the pack that was open and is now \nclosed?\n\nI don't suppose there would be an easy way to figure out the object it was \ntrying to unpack by applying the delta?\n\n> But then, the output pack-*.idx file created when\n> fast-import closed the pack would be wrong too.\n\nI think the wrong info it has is about the contents of a pack that had \nbeen closed previously. I think all of the info about objects in the open \npack is correct.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"104067","messageId":"20090210201203.GU30949@spearce.org","threadId":"17690","inReplyTo":"alpine.LNX.1.00.0902101427300.19665@iabervon.org","subject":"Re: fact-import: failed to apply delta","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-10T20:12:03Z","receivedAt":"2009-02-10T20:12:03Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Daniel Barkalow wrote:\n> \n> Is there some easy way to tell what object it was having problems with \n> when it failed to unpack? I've got a whole lot of objects.\n\nCan you use gdb to find it? If so, walk up the stack into\nfast-import.c's load_tree() function and look at sha1 here,\nand also, *myoe.\n \n> The expected size of the base is 1882, while the actual size is 151. The \n> base offset it found was 12.\n> \n> I'm using \"checkpoint\" a lot, so I've got 24 packs. Two of them have tree \n> objects of size 1882 at offset 12; a different one has a tree object of \n> size 151 at offset 12. The one with the object of size 151 was the one \n> that was still open at the end. There's no tree of size 1882 in this pack, \n> nor in any other pack that has a tree of size 151.\n> \n> So maybe it's right about the offsets and all, but it's confused about \n> which pack something was in? Maybe it cached something when the pack \n> containing the object it wants was open, and it ended up thinking it was \n> in the pack that's now open rather than the pack that was open and is now \n> closed?\n\nfast-import keeps all of its object data in a single table of\n\"struct object_entry\", the table is keyed by SHA-1. Each entry\nhas a pack_id, which tells it which pack this object is in, and\nthe offset of the object within that pack.\n\nSounds like maybe its confusing the pack pointer in the all_packs\narray (see gfi_unpack_entry).\n \n> I don't suppose there would be an easy way to figure out the object it was \n> trying to unpack by applying the delta?\n\nNot really. You'd have to write code for this. Or, since the pack\ncloses and you can index it, use \"verify-pack -v\" to find the object\nstarting at the offset you know its having trouble with, that should\ntell you the object's SHA-1.\n\n-- \nShawn.\n"},{"id":"104080","messageId":"alpine.LNX.1.00.0902101520240.19665@iabervon.org","threadId":"17690","inReplyTo":"20090210201203.GU30949@spearce.org","subject":"Re: fact-import: failed to apply delta","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-02-10T21:19:27Z","receivedAt":"2009-02-10T21:19:27Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 10 Feb 2009, Shawn O. Pearce wrote:\n\n> Daniel Barkalow wrote:\n> > \n> > Is there some easy way to tell what object it was having problems with \n> > when it failed to unpack? I've got a whole lot of objects.\n> \n> Can you use gdb to find it? If so, walk up the stack into\n> fast-import.c's load_tree() function and look at sha1 here,\n> and also, *myoe.\n\nOkay, sha1 is 961a2199..., which is correct as a delta against the tree of \nsize 151 at offset 12 of its own pack; the bogus thing seems to be the \nbase_size (and presumably base).\n\n> > The expected size of the base is 1882, while the actual size is 151. The \n> > base offset it found was 12.\n> > \n> > I'm using \"checkpoint\" a lot, so I've got 24 packs. Two of them have tree \n> > objects of size 1882 at offset 12; a different one has a tree object of \n> > size 151 at offset 12. The one with the object of size 151 was the one \n> > that was still open at the end. There's no tree of size 1882 in this pack, \n> > nor in any other pack that has a tree of size 151.\n> > \n> > So maybe it's right about the offsets and all, but it's confused about \n> > which pack something was in? Maybe it cached something when the pack \n> > containing the object it wants was open, and it ended up thinking it was \n> > in the pack that's now open rather than the pack that was open and is now \n> > closed?\n> \n> fast-import keeps all of its object data in a single table of\n> \"struct object_entry\", the table is keyed by SHA-1. Each entry\n> has a pack_id, which tells it which pack this object is in, and\n> the offset of the object within that pack.\n> \n> Sounds like maybe its confusing the pack pointer in the all_packs\n> array (see gfi_unpack_entry).\n\nI think maybe there's aliasing in the delta base cache? If it recycled a \nstruct packed_git, the cache would come up with a cached tree at offset 12 \nof the packed_git at that address, but the pack used by that struct has \nchanged.\n\nI don't see any reason that the situation couldn't arise where you start a\npack, look up an object in it while it's still open but the object isn't \nin the window, cache the delta base, end that packfile, eventually start a \npackfile that gets allocated in the space that was freed, produce a new \ndelta against the object at exactly the same offset of the new pack (with \nthe same address as the old pack), and go on happily until you try looking \nup this last delta and pull the wrong base out of the cache.\n\nI don't see any code to flush the delta cache ever, but it's hard to get a \nnew packed_git allocated at the address of a freed one, except by doing a \nlot of checkpoints in fast-import...\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"104084","messageId":"20090210212539.GV30949@spearce.org","threadId":"17690","inReplyTo":"alpine.LNX.1.00.0902101520240.19665@iabervon.org","subject":"Re: fact-import: failed to apply delta","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-10T21:25:39Z","receivedAt":"2009-02-10T21:25:39Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Daniel Barkalow wrote:\n> \n> I think maybe there's aliasing in the delta base cache? If it recycled a \n> struct packed_git, the cache would come up with a cached tree at offset 12 \n> of the packed_git at that address, but the pack used by that struct has \n> changed.\n\nYup, that must be it.\n \n> I don't see any reason that the situation couldn't arise where you start a\n> pack, look up an object in it while it's still open but the object isn't \n> in the window, cache the delta base, end that packfile, eventually start a \n> packfile that gets allocated in the space that was freed, produce a new \n> delta against the object at exactly the same offset of the new pack (with \n> the same address as the old pack), and go on happily until you try looking \n> up this last delta and pull the wrong base out of the cache.\n> \n> I don't see any code to flush the delta cache ever, but it's hard to get a \n> new packed_git allocated at the address of a freed one, except by doing a \n> lot of checkpoints in fast-import...\n\n*ouch*. I think you found it.\n\nWe should dump the cached_objects table in sha1_file.c during\na checkpoint in fast-import.\n\n-- \nShawn.\n"},{"id":"104085","messageId":"alpine.LNX.1.00.0902101628140.19665@iabervon.org","threadId":"17690","inReplyTo":"20090210212539.GV30949@spearce.org","subject":"Re: fact-import: failed to apply delta","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-02-10T21:32:11Z","receivedAt":"2009-02-10T21:32:11Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 10 Feb 2009, Shawn O. Pearce wrote:\n\n> Daniel Barkalow wrote:\n> > \n> > I think maybe there's aliasing in the delta base cache? If it recycled a \n> > struct packed_git, the cache would come up with a cached tree at offset 12 \n> > of the packed_git at that address, but the pack used by that struct has \n> > changed.\n> \n> Yup, that must be it.\n> \n> > I don't see any reason that the situation couldn't arise where you start a\n> > pack, look up an object in it while it's still open but the object isn't \n> > in the window, cache the delta base, end that packfile, eventually start a \n> > packfile that gets allocated in the space that was freed, produce a new \n> > delta against the object at exactly the same offset of the new pack (with \n> > the same address as the old pack), and go on happily until you try looking \n> > up this last delta and pull the wrong base out of the cache.\n> > \n> > I don't see any code to flush the delta cache ever, but it's hard to get a \n> > new packed_git allocated at the address of a freed one, except by doing a \n> > lot of checkpoints in fast-import...\n> \n> *ouch*. I think you found it.\n> \n> We should dump the cached_objects table in sha1_file.c during\n> a checkpoint in fast-import.\n\nNo, that one's keyed by sha1, and doesn't get collisions; it's the \ndelta_base_cache that's the issue; it's keyed by struct packed_git * and \noffset.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"104086","messageId":"20090210213612.GW30949@spearce.org","threadId":"17690","inReplyTo":"alpine.LNX.1.00.0902101628140.19665@iabervon.org","subject":"Re: fact-import: failed to apply delta","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-10T21:36:12Z","receivedAt":"2009-02-10T21:36:12Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Daniel Barkalow wrote:\n> On Tue, 10 Feb 2009, Shawn O. Pearce wrote:\n> > \n> > We should dump the cached_objects table in sha1_file.c during\n> > a checkpoint in fast-import.\n> \n> No, that one's keyed by sha1, and doesn't get collisions; it's the \n> delta_base_cache that's the issue; it's keyed by struct packed_git * and \n> offset.\n\nUh, yea, I realize that after I sent the message. Does this patch\nfix it for you?\n\n--8<--\nClear the delta base cache during fast-import checkpoint\n\nOtherwise we may reuse the same memory address for a totally\ndifferent \"struct packed_git\", and a previously cached object from\nthe prior occupant might be returned when trying to unpack an object\nfrom the new pack.\n\nFound-by: Daniel Barkalow \nSigned-off-by: Shawn O. Pearce \n---\n cache.h | 1 +\n fast-import.c | 1 +\n sha1_file.c | 7 +++++++\n 3 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 8dcc53c..7f1a6e8 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -830,6 +830,7 @@ extern unsigned char* use_pack(struct packed_git *, struct pack_window **, off_t\n extern void close_pack_windows(struct packed_git *);\n extern void unuse_pack(struct pack_window **);\n extern void free_pack_by_name(const char *);\n+extern void clear_delta_base_cache(void);\n extern struct packed_git *add_packed_git(const char *, int, int);\n extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t);\n extern off_t nth_packed_object_offset(const struct packed_git *, uint32_t);\ndiff --git a/fast-import.c b/fast-import.c\nindex 1935206..03b13e0 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -945,6 +945,7 @@ static void end_packfile(void)\n {\n \tstruct packed_git *old_p = pack_data, *new_p;\n \n+\tclear_delta_base_cache();\n \tif (object_count) {\n \t\tchar *idx_name;\n \t\tint i;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 8868b80..d2dbc96 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1663,6 +1663,13 @@ static inline void release_delta_base_cache(struct delta_base_cache_entry *ent)\n \t}\n }\n \n+void clear_delta_base_cache(void)\n+{\n+\tunsigned long p;\n+\tfor (p = 0; p < MAX_DELTA_CACHE; p++)\n+\t\trelease_delta_base_cache(&delta_base_cache[p])\n+}\n+\n static void add_delta_base_cache(struct packed_git *p, off_t base_offset,\n \tvoid *base, unsigned long base_size, enum object_type type)\n {\n-- \n1.6.2.rc0.186.g417c\n"},{"id":"104088","messageId":"alpine.LNX.1.00.0902101646530.19665@iabervon.org","threadId":"17690","inReplyTo":"20090210213612.GW30949@spearce.org","subject":"Re: fact-import: failed to apply delta","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-02-10T21:51:13Z","receivedAt":"2009-02-10T21:51:13Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 10 Feb 2009, Shawn O. Pearce wrote:\n\n> Daniel Barkalow wrote:\n> > On Tue, 10 Feb 2009, Shawn O. Pearce wrote:\n> > > \n> > > We should dump the cached_objects table in sha1_file.c during\n> > > a checkpoint in fast-import.\n> > \n> > No, that one's keyed by sha1, and doesn't get collisions; it's the \n> > delta_base_cache that's the issue; it's keyed by struct packed_git * and \n> > offset.\n> \n> Uh, yea, I realize that after I sent the message. Does this patch\n> fix it for you?\n\nAside from the trivial typo, yes. (Although I can't be 100% sure it didn't \njust happen to change things leading to needing a different test case; I \ncan say for sure that it got past the previous code's MTBF, which is a \ngood sign.)\n\n> --8<--\n> Clear the delta base cache during fast-import checkpoint\n> \n> Otherwise we may reuse the same memory address for a totally\n> different \"struct packed_git\", and a previously cached object from\n> the prior occupant might be returned when trying to unpack an object\n> from the new pack.\n> \n> Found-by: Daniel Barkalow \n> Signed-off-by: Shawn O. Pearce \n> ---\n> cache.h | 1 +\n> fast-import.c | 1 +\n> sha1_file.c | 7 +++++++\n> 3 files changed, 9 insertions(+), 0 deletions(-)\n> \n> diff --git a/cache.h b/cache.h\n> index 8dcc53c..7f1a6e8 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -830,6 +830,7 @@ extern unsigned char* use_pack(struct packed_git *, struct pack_window **, off_t\n> extern void close_pack_windows(struct packed_git *);\n> extern void unuse_pack(struct pack_window **);\n> extern void free_pack_by_name(const char *);\n> +extern void clear_delta_base_cache(void);\n> extern struct packed_git *add_packed_git(const char *, int, int);\n> extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t);\n> extern off_t nth_packed_object_offset(const struct packed_git *, uint32_t);\n> diff --git a/fast-import.c b/fast-import.c\n> index 1935206..03b13e0 100644\n> --- a/fast-import.c\n> +++ b/fast-import.c\n> @@ -945,6 +945,7 @@ static void end_packfile(void)\n> {\n> \tstruct packed_git *old_p = pack_data, *new_p;\n> \n> +\tclear_delta_base_cache();\n> \tif (object_count) {\n> \t\tchar *idx_name;\n> \t\tint i;\n> diff --git a/sha1_file.c b/sha1_file.c\n> index 8868b80..d2dbc96 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -1663,6 +1663,13 @@ static inline void release_delta_base_cache(struct delta_base_cache_entry *ent)\n> \t}\n> }\n> \n> +void clear_delta_base_cache(void)\n> +{\n> +\tunsigned long p;\n> +\tfor (p = 0; p < MAX_DELTA_CACHE; p++)\n> +\t\trelease_delta_base_cache(&delta_base_cache[p])\n> +}\n> +\n> static void add_delta_base_cache(struct packed_git *p, off_t base_offset,\n> \tvoid *base, unsigned long base_size, enum object_type type)\n> {\n> -- \n> 1.6.2.rc0.186.g417c\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n> \n"},{"id":"104093","messageId":"7vprhqkjrr.fsf@gitster.siamese.dyndns.org","threadId":"17690","inReplyTo":"20090210213612.GW30949@spearce.org","subject":"Re: fact-import: failed to apply delta","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-10T22:30:00Z","receivedAt":"2009-02-10T22:30:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> Daniel Barkalow wrote:\n>> On Tue, 10 Feb 2009, Shawn O. Pearce wrote:\n>> > \n>> > We should dump the cached_objects table in sha1_file.c during\n>> > a checkpoint in fast-import.\n>> \n>> No, that one's keyed by sha1, and doesn't get collisions; it's the \n>> delta_base_cache that's the issue; it's keyed by struct packed_git * and \n>> offset.\n>\n> Uh, yea, I realize that after I sent the message. Does this patch\n> fix it for you?\n>\n> --8<--\n> Clear the delta base cache during fast-import checkpoint\n>\n> Otherwise we may reuse the same memory address for a totally\n> different \"struct packed_git\", and a previously cached object from\n> the prior occupant might be returned when trying to unpack an object\n> from the new pack.\n\nCan this be made more automatic?\n\nFor example if you do this every time a new pack is installed to\nsha1_file(), like in add_packed_git() perhaps, wouldn't that be much less\nerror prone?\n"},{"id":"104097","messageId":"7vfxillxiu.fsf@gitster.siamese.dyndns.org","threadId":"17690","inReplyTo":"7vprhqkjrr.fsf@gitster.siamese.dyndns.org","subject":"Re: fact-import: failed to apply delta","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-10T22:47:37Z","receivedAt":"2009-02-10T22:47:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> \"Shawn O. Pearce\" writes:\n>\n>> Daniel Barkalow wrote:\n>>> On Tue, 10 Feb 2009, Shawn O. Pearce wrote:\n>>> > \n>>> > We should dump the cached_objects table in sha1_file.c during\n>>> > a checkpoint in fast-import.\n>>> \n>>> No, that one's keyed by sha1, and doesn't get collisions; it's the \n>>> delta_base_cache that's the issue; it's keyed by struct packed_git * and \n>>> offset.\n>>\n>> Uh, yea, I realize that after I sent the message. Does this patch\n>> fix it for you?\n>>\n>> --8<--\n>> Clear the delta base cache during fast-import checkpoint\n>>\n>> Otherwise we may reuse the same memory address for a totally\n>> different \"struct packed_git\", and a previously cached object from\n>> the prior occupant might be returned when trying to unpack an object\n>> from the new pack.\n>\n> Can this be made more automatic?\n>\n> For example if you do this every time a new pack is installed to\n> sha1_file(), like in add_packed_git() perhaps, wouldn't that be much less\n> error prone?\n\nOn second thought, I think fast-import is the only program that plays\nfunny games of feeding a packed_git that is *not* part of the real list of\npacked_git installed in the system to unpack_entry(), so probably your\npatch is a better idea.\n\nWe really need a better comment in the codepath from gfi_unpack_entry() to\nunpack_entry() that there is a very unusual thing going on.\n\nBy the way, strictly speaking, you need to release the delta_base_cache\nentries that is based on pack_data and nothing else, no?\n"},{"id":"104105","messageId":"20090210230950.GX30949@spearce.org","threadId":"17690","inReplyTo":"7vfxillxiu.fsf@gitster.siamese.dyndns.org","subject":"Re: fact-import: failed to apply delta","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-10T23:09:50Z","receivedAt":"2009-02-10T23:09:50Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> \n> On second thought, I think fast-import is the only program that plays\n> funny games of feeding a packed_git that is *not* part of the real list of\n> packed_git installed in the system to unpack_entry(), so probably your\n> patch is a better idea.\n\nRight, that was my thought.\n \n> We really need a better comment in the codepath from gfi_unpack_entry() to\n> unpack_entry() that there is a very unusual thing going on.\n\nThat whole code is hairy. It already has more comments than code.\nWhat more can I really say here other than maybe this?\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 03b13e0..7bfb563 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1204,6 +1204,12 @@ static void *gfi_unpack_entry(\n \t\t */\n \t\tp->pack_size = pack_size + 20;\n \t}\n+\t/* DANGER, WILL ROBINSON DANGER !!!!\n+\t *\n+\t * unpack_entry() wasn't meant to be called the way we are\n+\t * about to call it right here. Be very careful, any sort\n+\t * of assumption is probably wrong.\n+\t */\n \treturn unpack_entry(p, oe->offset, &type, sizep);\n }\n \n \n> By the way, strictly speaking, you need to release the delta_base_cache\n> entries that is based on pack_data and nothing else, no?\n\nRight.\n\nBut the hiccup of a checkpoint in terms of overall performance is\nsuch a huge amount (due to needing to re-read the entire pack to\ncompute its final checksum) that the loss of the delta_base_cache\nis pretty much a drop in the bucket here.\n\nI can go back and add in a struct packed_git* and filter to only\nthose entries in the cache, but it doesn't seem worth it to me.\n\n-- \nShawn.\n"},{"id":"104108","messageId":"7vtz71khoi.fsf@gitster.siamese.dyndns.org","threadId":"17690","inReplyTo":"20090210230950.GX30949@spearce.org","subject":"Re: fact-import: failed to apply delta","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-10T23:15:09Z","receivedAt":"2009-02-10T23:15:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> What more can I really say here other than maybe this?\n>\n> diff --git a/fast-import.c b/fast-import.c\n> index 03b13e0..7bfb563 100644\n> --- a/fast-import.c\n> +++ b/fast-import.c\n> @@ -1204,6 +1204,12 @@ static void *gfi_unpack_entry(\n> \t\t */\n> \t\tp->pack_size = pack_size + 20;\n> \t}\n> +\t/* DANGER, WILL ROBINSON DANGER !!!!\n> +\t *\n> +\t * unpack_entry() wasn't meant to be called the way we are\n> +\t * about to call it right here. Be very careful, any sort\n> +\t * of assumption is probably wrong.\n> +\t */\n> \treturn unpack_entry(p, oe->offset, &type, sizep);\n> }\n\nYuck ;-).\n\n>> By the way, strictly speaking, you need to release the delta_base_cache\n>> entries that is based on pack_data and nothing else, no?\n>\n> Right.\n>\n> But the hiccup of a checkpoint in terms of overall performance is\n> such a huge amount (due to needing to re-read the entire pack to\n> compute its final checksum) that the loss of the delta_base_cache\n> is pretty much a drop in the bucket here.\n>\n> I can go back and add in a struct packed_git* and filter to only\n> those entries in the cache, but it doesn't seem worth it to me.\n\nNah, that was not a suggestion but a question.\n\nThe patch can and should go to maint, right?\n"},{"id":"104109","messageId":"20090210231653.GY30949@spearce.org","threadId":"17690","inReplyTo":"7vtz71khoi.fsf@gitster.siamese.dyndns.org","subject":"Re: fact-import: failed to apply delta","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-10T23:16:53Z","receivedAt":"2009-02-10T23:16:53Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> \n> The patch can and should go to maint, right?\n\nYea, maint. Don't forget the ';' I forgot in sha1_file.c.\n\nClearly, I failed to compile-test it before sending. :-)\n\n-- \nShawn.\n"},{"id":"104111","messageId":"7vljsdkgwc.fsf@gitster.siamese.dyndns.org","threadId":"17690","inReplyTo":"20090210231653.GY30949@spearce.org","subject":"Re: fact-import: failed to apply delta","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-10T23:32:03Z","receivedAt":"2009-02-10T23:32:03Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> Junio C Hamano wrote:\n>> \n>> The patch can and should go to maint, right?\n>\n> Yea, maint. Don't forget the ';' I forgot in sha1_file.c.\n>\n> Clearly, I failed to compile-test it before sending. :-)\n\nThat's Ok, I always compile test before merging the result of \"git am\" to\nany other place ;-).\n"},{"id":"104236","messageId":"alpine.LNX.1.00.0902111247300.19665@iabervon.org","threadId":"17690","inReplyTo":"7vfxillxiu.fsf@gitster.siamese.dyndns.org","subject":"Re: fact-import: failed to apply delta","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-02-11T18:09:12Z","receivedAt":"2009-02-11T18:09:12Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 10 Feb 2009, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n> \n> > \"Shawn O. Pearce\" writes:\n> >\n> >> Daniel Barkalow wrote:\n> >>> On Tue, 10 Feb 2009, Shawn O. Pearce wrote:\n> >>> > \n> >>> > We should dump the cached_objects table in sha1_file.c during\n> >>> > a checkpoint in fast-import.\n> >>> \n> >>> No, that one's keyed by sha1, and doesn't get collisions; it's the \n> >>> delta_base_cache that's the issue; it's keyed by struct packed_git * and \n> >>> offset.\n> >>\n> >> Uh, yea, I realize that after I sent the message. Does this patch\n> >> fix it for you?\n> >>\n> >> --8<--\n> >> Clear the delta base cache during fast-import checkpoint\n> >>\n> >> Otherwise we may reuse the same memory address for a totally\n> >> different \"struct packed_git\", and a previously cached object from\n> >> the prior occupant might be returned when trying to unpack an object\n> >> from the new pack.\n> >\n> > Can this be made more automatic?\n> >\n> > For example if you do this every time a new pack is installed to\n> > sha1_file(), like in add_packed_git() perhaps, wouldn't that be much less\n> > error prone?\n> \n> On second thought, I think fast-import is the only program that plays\n> funny games of feeding a packed_git that is *not* part of the real list of\n> packed_git installed in the system to unpack_entry(), so probably your\n> patch is a better idea.\n\nThat's not the problem; the problem is calling free on a struct packed_git \nthat has been given to unpack_entry(), because it raises the possibility \nof having the memory allocated to a different pack in the future and \nending up with actively wrong entries in the delta cache, since it keys \noff of the pointer.\n\nI think free_pack_by_name() also needs to drop the entries that are from \nthe freed pack, to avoid having repack able to get the same problem, \nalthough I wouldn't be surprised if repack happened to never allocate a \nnew pack after freeing an old pack with stale delta cache entries, or \nnever used the delta cache after that, simply because it does one thing \nand then exits.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"104240","messageId":"20090211181530.GO30949@spearce.org","threadId":"17690","inReplyTo":"alpine.LNX.1.00.0902111247300.19665@iabervon.org","subject":"Re: fact-import: failed to apply delta","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-11T18:15:30Z","receivedAt":"2009-02-11T18:15:30Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Daniel Barkalow wrote:\n> \n> I think free_pack_by_name() also needs to drop the entries that are from \n> the freed pack, to avoid having repack able to get the same problem, \n> although I wouldn't be surprised if repack happened to never allocate a \n> new pack after freeing an old pack with stale delta cache entries, or \n> never used the delta cache after that, simply because it does one thing \n> and then exits.\n\nOy. I missed that we added this function. Patch follows.\n\n--8<--\nClear the delta base cache if a pack is rebuilt\n\nThere is some risk that re-opening a regenerated pack file with\ndifferent offsets could leave stale entries within the delta base\ncache that could be matched up against other objects using the same\n\"struct packed_git*\" and pack offset.\n\nThrowing away the entire delta base cache in this case is safer,\nas we don't have to worry about a recycled \"struct packed_git*\"\nmatching to the wrong base object, resulting in delta apply\nerrors while unpacking an object.\n\nSuggested-by: Daniel Barkalow \nSigned-off-by: Shawn O. Pearce \n---\n sha1_file.c | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 7459a9c..5b6e0f6 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -689,6 +689,7 @@ void free_pack_by_name(const char *pack_name)\n \twhile (*pp) {\n \t\tp = *pp;\n \t\tif (strcmp(pack_name, p->pack_name) == 0) {\n+\t\t\tclear_delta_base_cache();\n \t\t\tclose_pack_windows(p);\n \t\t\tif (p->pack_fd != -1)\n \t\t\t\tclose(p->pack_fd);\n-- \n1.6.2.rc0.186.g417c\n"},{"id":"104243","messageId":"7vwsbweshq.fsf@gitster.siamese.dyndns.org","threadId":"17690","inReplyTo":"20090211181530.GO30949@spearce.org","subject":"Re: fact-import: failed to apply delta","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-11T18:30:25Z","receivedAt":"2009-02-11T18:30:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" writes:\n\n> Daniel Barkalow wrote:\n>> \n>> I think free_pack_by_name() also needs to drop the entries that are from \n>> the freed pack, to avoid having repack able to get the same problem, \n>> although I wouldn't be surprised if repack happened to never allocate a \n>> new pack after freeing an old pack with stale delta cache entries, or \n>> never used the delta cache after that, simply because it does one thing \n>> and then exits.\n>\n> Oy. I missed that we added this function. Patch follows.\n\nThanks, both.\n"},{"id":"104244","messageId":"alpine.LNX.1.00.0902111323000.19665@iabervon.org","threadId":"17690","inReplyTo":"20090211181530.GO30949@spearce.org","subject":"Re: fact-import: failed to apply delta","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-02-11T18:33:21Z","receivedAt":"2009-02-11T18:33:21Z","isPatch":false,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 11 Feb 2009, Shawn O. Pearce wrote:\n\n> Daniel Barkalow wrote:\n> > \n> > I think free_pack_by_name() also needs to drop the entries that are from \n> > the freed pack, to avoid having repack able to get the same problem, \n> > although I wouldn't be surprised if repack happened to never allocate a \n> > new pack after freeing an old pack with stale delta cache entries, or \n> > never used the delta cache after that, simply because it does one thing \n> > and then exits.\n> \n> Oy. I missed that we added this function. Patch follows.\n\nI think it would be more clear to do something below (instead of the \noriginal patch); I think there's a better chance of authors knowing when \nto use this function than knowing when to use a function based on what it \nactually does, and there's a better chance that any future optimizations \nthat need to be flushed under the same conditions would get included.\n\n--8<--\nProvide a function to free a struct packed_git that may have been used\n\nWhen we look up entries in a pack, we sometimes cache the results. If a \nstruct packed_git is freed afterwards (and its memory could be allocated \nas a different struct packed_git later), we need to clear out anything \nthat may mis-recognize the pack.\n\nIn particular, we flush the delta cache.\n\nSigned-off-by: Daniel Barkalow \n\ndiff --git a/cache.h b/cache.h\nindex 8d965b8..ecc55cf 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -822,6 +822,7 @@ extern unsigned char* use_pack(struct packed_git *, struct pack_window **, off_t\n extern void close_pack_windows(struct packed_git *);\n extern void unuse_pack(struct pack_window **);\n extern void free_pack_by_name(const char *);\n+extern void free_used_pack(struct packed_git *);\n extern struct packed_git *add_packed_git(const char *, int, int);\n extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t);\n extern off_t nth_packed_object_offset(const struct packed_git *, uint32_t);\ndiff --git a/fast-import.c b/fast-import.c\nindex f0e08ac..8ec9a4e 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -987,7 +987,7 @@ static void end_packfile(void)\n \t\tclose(old_p->pack_fd);\n \t\tunlink(old_p->pack_name);\n \t}\n-\tfree(old_p);\n+\tfree_used_pack(old_p);\n \n \t/* We can't carry a delta across packfiles. */\n \tstrbuf_release(&last_blob.data);\ndiff --git a/sha1_file.c b/sha1_file.c\nindex fd4980d..5a45f51 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -696,7 +696,7 @@ void free_pack_by_name(const char *pack_name)\n \t\t\t\tmunmap((void *)p->index_data, p->index_size);\n \t\t\tfree(p->bad_object_sha1);\n \t\t\t*pp = p->next;\n-\t\t\tfree(p);\n+\t\t\tfree_used_pack(p);\n \t\t\treturn;\n \t\t}\n \t\tpp = &p->next;\n@@ -1663,6 +1663,14 @@ static inline void release_delta_base_cache(struct delta_base_cache_entry *ent)\n \t}\n }\n \n+void free_used_pack(struct packed_git *pack)\n+{\n+\tunsigned long p;\n+\tfor (p = 0; p < MAX_DELTA_CACHE; p++)\n+\t\trelease_delta_base_cache(&delta_base_cache[p]);\n+\tfree(pack);\n+}\n+\n static void add_delta_base_cache(struct packed_git *p, off_t base_offset,\n \tvoid *base, unsigned long base_size, enum object_type type)\n {\n"}]}