Commit add1743580149c3d1e570aafff3180cee216162e

Patrick Steinhardt 2019-05-24T15:24:26

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.

diff --git a/src/cache.c b/src/cache.c
index 2f5c8f9..3128e40 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 30b7816..16e7bc4 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