cache: fix cache eviction using deallocated key

When evicting cache entries, we first retrieve the object that is
to be evicted, delete the object and then finally delete the key
from the cache. In case where the cache eviction caused us to
free the cached object, though, its key will point to invalid
memory now when trying to remove it from the cache map. On my
system, this causes us to not properly remove the key from the
map, as its memory has been overwritten already and thus the key
lookup it will fail and we cannot delete it.

Fix this by only decrementing the refcount of the evictee after
we have removed it from our cache map. Add a test that caused a
segfault previous to that change.

2 files changed, 9 insertions, 2 deletions

diff --git a/src/cache.c b/src/cache.c
index 2f5c8f99b..3128e40c5 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -134,9 +134,8 @@ static void cache_evict_entries(git_cache *cache)
 
 		evict_count--;
 		evicted_memory += evict->size;
-		git_cached_obj_decref(evict);
-
 		git_oidmap_delete(cache->map, key);
+		git_cached_obj_decref(evict);
 	}
 
 	cache->used_memory -= evicted_memory;
diff --git a/tests/blame/simple.c b/tests/blame/simple.c
index 30b78168f..16e7bc400 100644
--- a/tests/blame/simple.c
+++ b/tests/blame/simple.c
@@ -203,6 +203,14 @@ void test_blame_simple__trivial_libgit2(void)
 	check_blame_hunk_index(g_repo, g_blame, 49, 60, 1, 0, "d12299fe", "src/git.h");
 }
 
+/* This was leading to segfaults on some systems during cache eviction. */
+void test_blame_simple__trivial_libgit2_under_cache_pressure(void)
+{
+	ssize_t old_max_storage = git_cache__max_storage;
+	git_cache__max_storage = 1024 * 1024;
+	test_blame_simple__trivial_libgit2();
+	git_cache__max_storage = old_max_storage;
+}
 
 /*
  * $ git blame -n b.txt -L 8