On Wed, 11 Feb 2009, Shawn O. Pearce wrote:

quoted text
> 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)
 {
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html