Re: fact-import: failed to apply delta
- From
Daniel Barkalow <barkalow@iabervon.org>
- Date
- Feb 11, 2009, 18:33 UTC
- Message-ID
- <alpine.LNX.1.00.0902111323000.19665@iabervon.org>
- In-Reply-To
- <20090211181530.GO30949@spearce.org>
On Wed, 11 Feb 2009, Shawn O. Pearce wrote:
Show 10 quoted lines
> Daniel Barkalow <barkalow@iabervon.org> wrote: > > > > 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. > > Oy. I missed that we added this function. Patch follows.
I think it would be more clear to do something below (instead of the original patch); I think there's a better chance of authors knowing when to use this function than knowing when to use a function based on what it actually does, and there's a better chance that any future optimizations that need to be flushed under the same conditions would get included.
--8<-- Provide a function to free a struct packed_git that may have been used
When we look up entries in a pack, we sometimes cache the results. If a struct packed_git is freed afterwards (and its memory could be allocated as a different struct packed_git later), we need to clear out anything that may mis-recognize the pack.
In particular, we flush the delta cache.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
diff --git a/cache.h b/cache.h index 8d965b8..ecc55cf 100644 --- a/cache.h +++ b/cache.h @@ -822,6 +822,7 @@ extern unsigned char* use_pack(struct packed_git *, struct pack_window **, off_t extern void close_pack_windows(struct packed_git *); extern void unuse_pack(struct pack_window **); extern void free_pack_by_name(const char *); +extern void free_used_pack(struct packed_git *); extern struct packed_git *add_packed_git(const char *, int, int); extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t); extern off_t nth_packed_object_offset(const struct packed_git *, uint32_t); diff --git a/fast-import.c b/fast-import.c index f0e08ac..8ec9a4e 100644 --- a/fast-import.c +++ b/fast-import.c @@ -987,7 +987,7 @@ static void end_packfile(void) close(old_p->pack_fd); unlink(old_p->pack_name); } - free(old_p); + free_used_pack(old_p); /* We can't carry a delta across packfiles. */ strbuf_release(&last_blob.data); diff --git a/sha1_file.c b/sha1_file.c index fd4980d..5a45f51 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -696,7 +696,7 @@ void free_pack_by_name(const char *pack_name) munmap((void *)p->index_data, p->index_size); free(p->bad_object_sha1); *pp = p->next; - free(p); + free_used_pack(p); return; } pp = &p->next; @@ -1663,6 +1663,14 @@ static inline void release_delta_base_cache(struct delta_base_cache_entry *ent) } } +void free_used_pack(struct packed_git *pack) +{ + unsigned long p; + for (p = 0; p < MAX_DELTA_CACHE; p++) + release_delta_base_cache(&delta_base_cache[p]); + free(pack); +} + static void add_delta_base_cache(struct packed_git *p, off_t base_offset, void *base, unsigned long base_size, enum object_type type) {