Add diff threading tests and attr file cache locks

This adds a basic test of doing simultaneous diffs on multiple
threads and adds basic locking for the attr file cache because
that was the immediate problem that arose from these tests.

13 files changed, 361 insertions, 140 deletions

diff --git a/src/attr.c b/src/attr.c
index d8a171d0f..f52a8a97b 100644
--- a/src/attr.c
+++ b/src/attr.c
@@ -33,6 +33,7 @@ static int collect_attr_files(
 	const char *path,
 	git_vector *files);
 
+static void release_attr_files(git_vector *files);
 
 int git_attr_get(
 	const char **value,
@@ -76,7 +77,7 @@ int git_attr_get(
 	}
 
 cleanup:
-	git_vector_free(&files);
+	release_attr_files(&files);
 	git_attr_path__free(&path);
 
 	return error;
@@ -152,7 +153,7 @@ int git_attr_get_many(
 	}
 
 cleanup:
-	git_vector_free(&files);
+	release_attr_files(&files);
 	git_attr_path__free(&path);
 	git__free(info);
 
@@ -181,12 +182,10 @@ int git_attr_foreach(
 	if (git_attr_path__init(&path, pathname, git_repository_workdir(repo)) < 0)
 		return -1;
 
-	if ((error = collect_attr_files(repo, flags, pathname, &files)) < 0)
+	if ((error = collect_attr_files(repo, flags, pathname, &files)) < 0 ||
+		(error = git_strmap_alloc(&seen)) < 0)
 		goto cleanup;
 
-	seen = git_strmap_alloc();
-	GITERR_CHECK_ALLOC(seen);
-
 	git_vector_foreach(&files, i, file) {
 
 		git_attr_file__foreach_matching_rule(file, &path, j, rule) {
@@ -211,7 +210,7 @@ int git_attr_foreach(
 
 cleanup:
 	git_strmap_free(seen);
-	git_vector_free(&files);
+	release_attr_files(&files);
 	git_attr_path__free(&path);
 
 	return error;
@@ -350,12 +349,21 @@ static int load_attr_from_cache(
 	if (git_buf_printf(&cache_key, "%d#%s", (int)source, relative_path) < 0)
 		return -1;
 
-	cache_pos = git_strmap_lookup_index(cache->files, cache_key.ptr);
+	if (git_mutex_lock(&cache->lock) < 0) {
+		giterr_set(GITERR_OS, "Could not get cache attr lock");
+		git_buf_free(&cache_key);
+		return -1;
+	}
 
-	git_buf_free(&cache_key);
+	cache_pos = git_strmap_lookup_index(cache->files, cache_key.ptr);
 
-	if (git_strmap_valid_index(cache->files, cache_pos))
+	if (git_strmap_valid_index(cache->files, cache_pos)) {
 		*file = git_strmap_value_at(cache->files, cache_pos);
+		GIT_REFCOUNT_INC(*file);
+	}
+
+	git_mutex_unlock(&cache->lock);
+	git_buf_free(&cache_key);
 
 	return 0;
 }
@@ -367,20 +375,26 @@ int git_attr_cache__internal_file(
 {
 	int error = 0;
 	git_attr_cache *cache = git_repository_attr_cache(repo);
-	khiter_t cache_pos = git_strmap_lookup_index(cache->files, filename);
+	khiter_t cache_pos;
+
+	if (git_mutex_lock(&cache->lock) < 0) {
+		giterr_set(GITERR_OS, "Unable to get attr cache lock");
+		return -1;
+	}
+
+	cache_pos = git_strmap_lookup_index(cache->files, filename);
 
 	if (git_strmap_valid_index(cache->files, cache_pos)) {
 		*file = git_strmap_value_at(cache->files, cache_pos);
-		return 0;
 	}
+	else if (!(error = git_attr_file__new(file, 0, filename, &cache->pool))) {
 
-	if (git_attr_file__new(file, 0, filename, &cache->pool) < 0)
-		return -1;
-
-	git_strmap_insert(cache->files, (*file)->key + 2, *file, error);
-	if (error > 0)
-		error = 0;
+		git_strmap_insert(cache->files, (*file)->key + 2, *file, error);
+		if (error > 0)
+			error = 0;
+	}
 
+	git_mutex_unlock(&cache->lock);
 	return error;
 }
 
@@ -452,9 +466,17 @@ int git_attr_cache__push_file(
 	if (parse && (error = parse(repo, parsedata, content, file)) < 0)
 		goto finish;
 
-	git_strmap_insert(cache->files, file->key, file, error); //-V595
-	if (error > 0)
-		error = 0;
+	if (git_mutex_lock(&cache->lock) < 0) {
+		giterr_set(GITERR_OS, "Unable to get attr cache lock");
+		error = -1;
+	} else {
+		git_strmap_insert(cache->files, file->key, file, error); /* -V595 */
+		if (error > 0) { /* > 0 means inserting for the first time */
+			error = 0;
+			GIT_REFCOUNT_INC(file);
+		}
+		git_mutex_unlock(&cache->lock);
+	}
 
 	/* remember "cache buster" file signature */
 	if (blob)
@@ -481,7 +503,8 @@ finish:
 }
 
 #define push_attr_file(R,S,B,F) \
-	git_attr_cache__push_file((R),(B),(F),GIT_ATTR_FILE_FROM_FILE,git_attr_file__parse_buffer,NULL,(S))
+	git_attr_cache__push_file											\
+	((R),(B),(F),GIT_ATTR_FILE_FROM_FILE,git_attr_file__parse_buffer,NULL,(S))
 
 typedef struct {
 	git_repository *repo;
@@ -535,6 +558,18 @@ static int push_one_attr(void *ref, git_buf *path)
 	return error;
 }
 
+static void release_attr_files(git_vector *files)
+{
+	size_t i;
+	git_attr_file *file;
+
+	git_vector_foreach(files, i, file) {
+		git_attr_file__free(file);
+		files->contents[i] = NULL;
+	}
+	git_vector_free(files);
+}
+
 static int collect_attr_files(
 	git_repository *repo,
 	uint32_t flags,
@@ -600,7 +635,7 @@ static int collect_attr_files(
 
  cleanup:
 	if (error < 0)
-		git_vector_free(files);
+		release_attr_files(files);
 	git_buf_free(&dir);
 
 	return error;
@@ -637,61 +672,11 @@ static int attr_cache__lookup_path(
 	return error;
 }
 
-int git_attr_cache__init(git_repository *repo)
-{
-	int ret;
-	git_attr_cache *cache = git_repository_attr_cache(repo);
-	git_config *cfg;
-
-	if (cache->initialized)
-		return 0;
-
-	/* cache config settings for attributes and ignores */
-	if (git_repository_config__weakptr(&cfg, repo) < 0)
-		return -1;
-
-	ret = attr_cache__lookup_path(
-		&cache->cfg_attr_file, cfg, GIT_ATTR_CONFIG, GIT_ATTR_FILE_XDG);
-	if (ret < 0)
-		return ret;
-
-	ret = attr_cache__lookup_path(
-		&cache->cfg_excl_file, cfg, GIT_IGNORE_CONFIG, GIT_IGNORE_FILE_XDG);
-	if (ret < 0)
-		return ret;
-
-	/* allocate hashtable for attribute and ignore file contents */
-	if (cache->files == NULL) {
-		cache->files = git_strmap_alloc();
-		GITERR_CHECK_ALLOC(cache->files);
-	}
-
-	/* allocate hashtable for attribute macros */
-	if (cache->macros == NULL) {
-		cache->macros = git_strmap_alloc();
-		GITERR_CHECK_ALLOC(cache->macros);
-	}
-
-	/* allocate string pool */
-	if (git_pool_init(&cache->pool, 1, 0) < 0)
-		return -1;
-
-	cache->initialized = 1;
-
-	/* insert default macros */
-	return git_attr_add_macro(repo, "binary", "-diff -crlf -text");
-}
-
-void git_attr_cache_flush(
-	git_repository *repo)
+static void attr_cache__free(git_attr_cache *cache)
 {
-	git_attr_cache *cache;
-
-	if (!repo)
+	if (!cache)
 		return;
 
-	cache = git_repository_attr_cache(repo);
-
 	if (cache->files != NULL) {
 		git_attr_file *file;
 
@@ -720,19 +705,93 @@ void git_attr_cache_flush(
 	git__free(cache->cfg_excl_file);
 	cache->cfg_excl_file = NULL;
 
-	cache->initialized = 0;
+	git_mutex_free(&cache->lock);
+
+	git__free(cache);
+}
+
+int git_attr_cache__init(git_repository *repo)
+{
+	int ret = 0;
+	git_attr_cache *cache = git_repository_attr_cache(repo);
+	git_config *cfg;
+
+	if (cache)
+		return 0;
+
+	if ((ret = git_repository_config__weakptr(&cfg, repo)) < 0)
+		return ret;
+
+	cache = git__calloc(1, sizeof(git_attr_cache));
+	GITERR_CHECK_ALLOC(cache);
+
+	/* set up lock */
+	if (git_mutex_init(&cache->lock) < 0) {
+		giterr_set(GITERR_OS, "Unable to initialize lock for attr cache");
+		git__free(cache);
+		return -1;
+	}
+
+	/* cache config settings for attributes and ignores */
+	ret = attr_cache__lookup_path(
+		&cache->cfg_attr_file, cfg, GIT_ATTR_CONFIG, GIT_ATTR_FILE_XDG);
+	if (ret < 0)
+		goto cancel;
+
+	ret = attr_cache__lookup_path(
+		&cache->cfg_excl_file, cfg, GIT_IGNORE_CONFIG, GIT_IGNORE_FILE_XDG);
+	if (ret < 0)
+		goto cancel;
+
+	/* allocate hashtable for attribute and ignore file contents,
+	 * hashtable for attribute macros, and string pool
+	 */
+	if ((ret = git_strmap_alloc(&cache->files)) < 0 ||
+		(ret = git_strmap_alloc(&cache->macros)) < 0 ||
+		(ret = git_pool_init(&cache->pool, 1, 0)) < 0)
+		goto cancel;
+
+	cache = git__compare_and_swap(&repo->attrcache, NULL, cache);
+	if (cache)
+		goto cancel; /* raced with another thread, free this but no error */
+
+	/* insert default macros */
+	return git_attr_add_macro(repo, "binary", "-diff -crlf -text");
+
+cancel:
+	attr_cache__free(cache);
+	return ret;
+}
+
+void git_attr_cache_flush(git_repository *repo)
+{
+	git_attr_cache *cache;
+
+	/* this could be done less expensively, but for now, we'll just free
+	 * the entire attrcache and let the next use reinitialize it...
+	 */
+	if (repo && (cache = git__swap(repo->attrcache, NULL)) != NULL)
+		attr_cache__free(cache);
 }
 
 int git_attr_cache__insert_macro(git_repository *repo, git_attr_rule *macro)
 {
-	git_strmap *macros = git_repository_attr_cache(repo)->macros;
+	git_attr_cache *cache = git_repository_attr_cache(repo);
+	git_strmap *macros = cache->macros;
 	int error;
 
 	/* TODO: generate warning log if (macro->assigns.length == 0) */
 	if (macro->assigns.length == 0)
 		return 0;
 
-	git_strmap_insert(macros, macro->match.pattern, macro, error);
+	if (git_mutex_lock(&cache->lock) < 0) {
+		giterr_set(GITERR_OS, "Unable to get attr cache lock");
+		error = -1;
+	} else {
+		git_strmap_insert(macros, macro->match.pattern, macro, error);
+		git_mutex_unlock(&cache->lock);
+	}
+
 	return (error < 0) ? -1 : 0;
 }
 
diff --git a/src/attr_file.c b/src/attr_file.c
index ea92336f7..695f661a8 100644
--- a/src/attr_file.c
+++ b/src/attr_file.c
@@ -23,6 +23,7 @@ int git_attr_file__new(
 
 	attrs = git__calloc(1, sizeof(git_attr_file));
 	GITERR_CHECK_ALLOC(attrs);
+	GIT_REFCOUNT_INC(attrs);
 
 	if (pool)
 		attrs->pool = pool;
@@ -152,11 +153,8 @@ void git_attr_file__clear_rules(git_attr_file *file)
 	git_vector_free(&file->rules);
 }
 
-void git_attr_file__free(git_attr_file *file)
+static void attr_file_free(git_attr_file *file)
 {
-	if (!file)
-		return;
-
 	git_attr_file__clear_rules(file);
 
 	if (file->pool_is_allocated) {
@@ -168,6 +166,13 @@ void git_attr_file__free(git_attr_file *file)
 	git__free(file);
 }
 
+void git_attr_file__free(git_attr_file *file)
+{
+	if (!file)
+		return;
+	GIT_REFCOUNT_DEC(file, attr_file_free);
+}
+
 uint32_t git_attr_file__name_hash(const char *name)
 {
 	uint32_t h = 5381;
diff --git a/src/attr_file.h b/src/attr_file.h
index 3bc7c6cb8..dbd6696c9 100644
--- a/src/attr_file.h
+++ b/src/attr_file.h
@@ -64,6 +64,7 @@ typedef struct {
 } git_attr_assignment;
 
 typedef struct {
+	git_refcount rc;
 	char *key;				/* cache "source#path" this was loaded from */
 	git_vector rules;		/* vector of <rule*> or <fnmatch*> */
 	git_pool *pool;
diff --git a/src/attrcache.h b/src/attrcache.h
index 077633b87..4f9cff6bb 100644
--- a/src/attrcache.h
+++ b/src/attrcache.h
@@ -11,12 +11,12 @@
 #include "strmap.h"
 
 typedef struct {
-	int initialized;
-	git_pool pool;
-	git_strmap *files;	 /* hash path to git_attr_file of rules */
-	git_strmap *macros;	 /* hash name to vector<git_attr_assignment> */
 	char *cfg_attr_file; /* cached value of core.attributesfile */
 	char *cfg_excl_file; /* cached value of core.excludesfile */
+	git_strmap *files;	 /* hash path to git_attr_file of rules */
+	git_strmap *macros;	 /* hash name to vector<git_attr_assignment> */
+	git_mutex lock;
+	git_pool  pool;
 } git_attr_cache;
 
 extern int git_attr_cache__init(git_repository *repo);
diff --git a/src/config_file.c b/src/config_file.c
index aedf2cb12..bb26aa8a3 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -180,11 +180,15 @@ static int config_open(git_config_backend *cfg, git_config_level_t level)
 
 	b->level = level;
 
-	b->values = git_strmap_alloc();
-	GITERR_CHECK_ALLOC(b->values);
+	if ((res = git_strmap_alloc(&b->values)) < 0)
+		return res;
 
 	git_array_init(b->readers);
 	reader = git_array_alloc(b->readers);
+	if (!reader) {
+		git_strmap_free(b->values);
+		return -1;
+	}
 	memset(reader, 0, sizeof(struct reader));
 
 	reader->file_path = git__strdup(b->file_path);
@@ -205,6 +209,7 @@ static int config_open(git_config_backend *cfg, git_config_level_t level)
 
 	reader = git_array_get(b->readers, 0);
 	git_buf_free(&reader->buffer);
+
 	return res;
 }
 
@@ -218,8 +223,10 @@ static int config_refresh(git_config_backend *cfg)
 
 	for (i = 0; i < git_array_size(b->readers); i++) {
 		reader = git_array_get(b->readers, i);
+
 		res = git_futils_readbuffer_updated(
-			&reader->buffer, reader->file_path, &reader->file_mtime, &reader->file_size, &updated);
+			&reader->buffer, reader->file_path,
+			&reader->file_mtime, &reader->file_size, &updated);
 
 		if (res < 0)
 			return (res == GIT_ENOTFOUND) ? 0 : res;
@@ -233,10 +240,9 @@ static int config_refresh(git_config_backend *cfg)
 
 	/* need to reload - store old values and prep for reload */
 	old_values = b->values;
-	b->values = git_strmap_alloc();
-	GITERR_CHECK_ALLOC(b->values);
-
-	if ((res = config_parse(b, reader, b->level, 0)) < 0) {
+	if ((res = git_strmap_alloc(&b->values)) < 0) {
+		b->values = old_values;
+	} else if ((res = config_parse(b, reader, b->level, 0)) < 0) {
 		free_vars(b->values);
 		b->values = old_values;
 	} else {
diff --git a/src/diff_driver.c b/src/diff_driver.c
index 4c9a0af65..8136e0dd9 100644
--- a/src/diff_driver.c
+++ b/src/diff_driver.c
@@ -66,7 +66,7 @@ git_diff_driver_registry *git_diff_driver_registry_new()
 	if (!reg)
 		return NULL;
 
-	if ((reg->drivers = git_strmap_alloc()) == NULL) {
+	if (git_strmap_alloc(&reg->drivers) < 0) {
 		git_diff_driver_registry_free(reg);
 		return NULL;
 	}
diff --git a/src/repository.h b/src/repository.h
index 99923b63b..86db488fd 100644
--- a/src/repository.h
+++ b/src/repository.h
@@ -108,7 +108,7 @@ struct git_repository {
 	git_submodule_cache *_submodules;
 
 	git_cache objects;
-	git_attr_cache attrcache;
+	git_attr_cache *attrcache;
 	git_diff_driver_registry *diff_drivers;
 
 	char *path_repository;
@@ -123,7 +123,7 @@ struct git_repository {
 
 GIT_INLINE(git_attr_cache *) git_repository_attr_cache(git_repository *repo)
 {
-	return &repo->attrcache;
+	return repo->attrcache;
 }
 
 int git_repository_head_tree(git_tree **tree, git_repository *repo);
diff --git a/src/sortedcache.c b/src/sortedcache.c
index 13f0921f1..625322034 100644
--- a/src/sortedcache.c
+++ b/src/sortedcache.c
@@ -20,7 +20,7 @@ int git_sortedcache_new(
 
 	if (git_pool_init(&sc->pool, 1, 0) < 0 ||
 		git_vector_init(&sc->items, 4, item_cmp) < 0 ||
-		(sc->map = git_strmap_alloc()) == NULL)
+		git_strmap_alloc(&sc->map) < 0)
 		goto fail;
 
 	if (git_rwlock_init(&sc->lock)) {
@@ -39,8 +39,7 @@ int git_sortedcache_new(
 	return 0;
 
 fail:
-	if (sc->map)
-		git_strmap_free(sc->map);
+	git_strmap_free(sc->map);
 	git_vector_free(&sc->items);
 	git_pool_clear(&sc->pool);
 	git__free(sc);
diff --git a/src/strmap.h b/src/strmap.h
index 8276ab468..8985aaf7e 100644
--- a/src/strmap.h
+++ b/src/strmap.h
@@ -22,7 +22,9 @@ typedef khiter_t git_strmap_iter;
 #define GIT__USE_STRMAP \
 	__KHASH_IMPL(str, static kh_inline, const char *, void *, 1, kh_str_hash_func, kh_str_hash_equal)
 
-#define git_strmap_alloc()  kh_init(str)
+#define git_strmap_alloc(hp) \
+	((*(hp) = kh_init(str)) == NULL) ? giterr_set_oom(), -1 : 0
+
 #define git_strmap_free(h)  kh_destroy(str, h), h = NULL
 #define git_strmap_clear(h) kh_clear(str, h)
 
diff --git a/src/submodule.c b/src/submodule.c
index bea096df5..95d3d0d9c 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -1638,8 +1638,7 @@ static int submodule_cache_alloc(
 		return -1;
 	}
 
-	cache->submodules = git_strmap_alloc();
-	if (!cache->submodules) {
+	if (git_strmap_alloc(&cache->submodules) < 0) {
 		submodule_cache_free(cache);
 		return -1;
 	}
diff --git a/tests/clar_libgit2.h b/tests/clar_libgit2.h
index c2489db38..d395bd66f 100644
--- a/tests/clar_libgit2.h
+++ b/tests/clar_libgit2.h
@@ -11,7 +11,7 @@
  *
  * Use this wrapper around all `git_` library calls that return error codes!
  */
-#define cl_git_pass(expr) cl_git_pass_(expr, __FILE__, __LINE__)
+#define cl_git_pass(expr) cl_git_pass_((expr), __FILE__, __LINE__)
 
 #define cl_git_pass_(expr, file, line) do { \
 	int _lg2_error; \
diff --git a/tests/core/strmap.c b/tests/core/strmap.c
index f34a4f89f..a120f1feb 100644
--- a/tests/core/strmap.c
+++ b/tests/core/strmap.c
@@ -3,12 +3,22 @@
 
 GIT__USE_STRMAP;
 
+git_strmap *g_table;
+
+void test_core_strmap__initialize(void)
+{
+	cl_git_pass(git_strmap_alloc(&g_table));
+	cl_assert(g_table != NULL);
+}
+
+void test_core_strmap__cleanup(void)
+{
+	git_strmap_free(g_table);
+}
+
 void test_core_strmap__0(void)
 {
-	git_strmap *table = git_strmap_alloc();
-	cl_assert(table != NULL);
-	cl_assert(git_strmap_num_entries(table) == 0);
-	git_strmap_free(table);
+	cl_assert(git_strmap_num_entries(g_table) == 0);
 }
 
 static void insert_strings(git_strmap *table, int count)
@@ -37,21 +47,17 @@ void test_core_strmap__1(void)
 {
 	int i;
 	char *str;
-	git_strmap *table = git_strmap_alloc();
-	cl_assert(table != NULL);
 
-	insert_strings(table, 20);
+	insert_strings(g_table, 20);
 
-	cl_assert(git_strmap_exists(table, "aaaaaaaaa"));
-	cl_assert(git_strmap_exists(table, "ggggggggg"));
-	cl_assert(!git_strmap_exists(table, "aaaaaaaab"));
-	cl_assert(!git_strmap_exists(table, "abcdefghi"));
+	cl_assert(git_strmap_exists(g_table, "aaaaaaaaa"));
+	cl_assert(git_strmap_exists(g_table, "ggggggggg"));
+	cl_assert(!git_strmap_exists(g_table, "aaaaaaaab"));
+	cl_assert(!git_strmap_exists(g_table, "abcdefghi"));
 
 	i = 0;
-	git_strmap_foreach_value(table, str, { i++; free(str); });
+	git_strmap_foreach_value(g_table, str, { i++; free(str); });
 	cl_assert(i == 20);
-
-	git_strmap_free(table);
 }
 
 void test_core_strmap__2(void)
@@ -59,44 +65,36 @@ void test_core_strmap__2(void)
 	khiter_t pos;
 	int i;
 	char *str;
-	git_strmap *table = git_strmap_alloc();
-	cl_assert(table != NULL);
 
-	insert_strings(table, 20);
+	insert_strings(g_table, 20);
 
-	cl_assert(git_strmap_exists(table, "aaaaaaaaa"));
-	cl_assert(git_strmap_exists(table, "ggggggggg"));
-	cl_assert(!git_strmap_exists(table, "aaaaaaaab"));
-	cl_assert(!git_strmap_exists(table, "abcdefghi"));
+	cl_assert(git_strmap_exists(g_table, "aaaaaaaaa"));
+	cl_assert(git_strmap_exists(g_table, "ggggggggg"));
+	cl_assert(!git_strmap_exists(g_table, "aaaaaaaab"));
+	cl_assert(!git_strmap_exists(g_table, "abcdefghi"));
 
-	cl_assert(git_strmap_exists(table, "bbbbbbbbb"));
-	pos = git_strmap_lookup_index(table, "bbbbbbbbb");
-	cl_assert(git_strmap_valid_index(table, pos));
-	cl_assert_equal_s(git_strmap_value_at(table, pos), "bbbbbbbbb");
-	free(git_strmap_value_at(table, pos));
-	git_strmap_delete_at(table, pos);
+	cl_assert(git_strmap_exists(g_table, "bbbbbbbbb"));
+	pos = git_strmap_lookup_index(g_table, "bbbbbbbbb");
+	cl_assert(git_strmap_valid_index(g_table, pos));
+	cl_assert_equal_s(git_strmap_value_at(g_table, pos), "bbbbbbbbb");
+	free(git_strmap_value_at(g_table, pos));
+	git_strmap_delete_at(g_table, pos);
 
-	cl_assert(!git_strmap_exists(table, "bbbbbbbbb"));
+	cl_assert(!git_strmap_exists(g_table, "bbbbbbbbb"));
 
 	i = 0;
-	git_strmap_foreach_value(table, str, { i++; free(str); });
+	git_strmap_foreach_value(g_table, str, { i++; free(str); });
 	cl_assert(i == 19);
-
-	git_strmap_free(table);
 }
 
 void test_core_strmap__3(void)
 {
 	int i;
 	char *str;
-	git_strmap *table = git_strmap_alloc();
-	cl_assert(table != NULL);
 
-	insert_strings(table, 10000);
+	insert_strings(g_table, 10000);
 
 	i = 0;
-	git_strmap_foreach_value(table, str, { i++; free(str); });
+	git_strmap_foreach_value(g_table, str, { i++; free(str); });
 	cl_assert(i == 10000);
-
-	git_strmap_free(table);
 }
diff --git a/tests/threads/diff.c b/tests/threads/diff.c
new file mode 100644
index 000000000..33afc58ac
--- /dev/null
+++ b/tests/threads/diff.c
@@ -0,0 +1,152 @@
+#include "clar_libgit2.h"
+#include "thread-utils.h"
+
+static git_repository *g_repo;
+static git_tree *a, *b;
+static git_atomic counts[4];
+
+void test_threads_diff__cleanup(void)
+{
+	cl_git_sandbox_cleanup();
+}
+
+static void run_in_parallel(
+	int repeats, int threads, void *(*func)(void *),
+	void (*before_test)(void), void (*after_test)(void))
+{
+	int r, t, *id = git__calloc(threads, sizeof(int));
+#ifdef GIT_THREADS
+	git_thread *th = git__calloc(threads, sizeof(git_thread));
+#else
+	void *th = NULL;
+#endif
+
+	cl_assert(id != NULL && th != NULL);
+
+	for (r = 0; r < repeats; ++r) {
+		g_repo = cl_git_sandbox_reopen(); /* reopen sandbox to flush caches */
+
+		if (before_test) before_test();
+
+		for (t = 0; t < threads; ++t) {
+			id[t] = t;
+#ifdef GIT_THREADS
+			cl_git_pass(git_thread_create(&th[t], NULL, func, &id[t]));
+#else
+			cl_assert(func(&id[t]) == &id[t]);
+#endif
+		}
+
+#ifdef GIT_THREADS
+		for (t = 0; t < threads; ++t)
+			cl_git_pass(git_thread_join(th[t], NULL));
+		memset(th, 0, threads * sizeof(git_thread));
+#endif
+
+		if (after_test) after_test();
+	}
+
+	git__free(id);
+	git__free(th);
+}
+
+static void setup_trees(void)
+{
+	cl_git_pass(git_revparse_single(
+		(git_object **)&a, g_repo, "0017bd4ab1^{tree}"));
+	cl_git_pass(git_revparse_single(
+		(git_object **)&b, g_repo, "26a125ee1b^{tree}"));
+
+	memset(counts, 0, sizeof(counts));
+}
+
+#define THREADS 20
+
+static void free_trees(void)
+{
+	git_tree_free(a); a = NULL;
+	git_tree_free(b); b = NULL;
+
+	cl_assert_equal_i(288, git_atomic_get(&counts[0]));
+	cl_assert_equal_i(112, git_atomic_get(&counts[1]));
+	cl_assert_equal_i( 80, git_atomic_get(&counts[2]));
+	cl_assert_equal_i( 96, git_atomic_get(&counts[3]));
+}
+
+static void *run_index_diffs(void *arg)
+{
+	int thread = *(int *)arg;
+	git_diff_options opts = GIT_DIFF_OPTIONS_INIT;
+	git_diff *diff = NULL;
+	size_t i;
+	int exp[4] = { 0, 0, 0, 0 };
+
+//	fprintf(stderr, "%d >>>\n", thread);
+
+	switch (thread & 0x03) {
+	case 0: /* diff index to workdir */;
+		cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts));
+		break;
+	case 1: /* diff tree 'a' to index */;
+		cl_git_pass(git_diff_tree_to_index(&diff, g_repo, a, NULL, &opts));
+		break;
+	case 2: /* diff tree 'b' to index */;
+		cl_git_pass(git_diff_tree_to_index(&diff, g_repo, b, NULL, &opts));
+		break;
+	case 3: /* diff index to workdir (explicit index) */;
+		{
+			git_index *idx;
+			cl_git_pass(git_repository_index(&idx, g_repo));
+			cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, idx, &opts));
+			git_index_free(idx);
+			break;
+		}
+	}
+
+//	fprintf(stderr, "%d <<<\n", thread);
+
+	/* keep some diff stats to make sure results are as expected */
+
+	i = git_diff_num_deltas(diff);
+	git_atomic_add(&counts[0], (int32_t)i);
+	exp[0] = (int)i;
+
+	while (i > 0) {
+		switch (git_diff_get_delta(diff, --i)->status) {
+		case GIT_DELTA_MODIFIED: exp[1]++; git_atomic_inc(&counts[1]); break;
+		case GIT_DELTA_ADDED:    exp[2]++; git_atomic_inc(&counts[2]); break;
+		case GIT_DELTA_DELETED:  exp[3]++; git_atomic_inc(&counts[3]); break;
+		default: break;
+		}
+	}
+
+//	fprintf(stderr, "%2d: [%d] total %d (M %d A %d D %d)\n",
+//			thread, (int)(thread & 0x03), exp[0], exp[1], exp[2], exp[3]);
+
+	switch (thread & 0x03) {
+	case 0: case 3:
+		cl_assert_equal_i(8, exp[0]); cl_assert_equal_i(4, exp[1]);
+		cl_assert_equal_i(0, exp[2]); cl_assert_equal_i(4, exp[3]);
+		break;
+	case 1:
+		cl_assert_equal_i(12, exp[0]); cl_assert_equal_i(3, exp[1]);
+		cl_assert_equal_i(7, exp[2]); cl_assert_equal_i(2, exp[3]);
+		break;
+	case 2:
+		cl_assert_equal_i(8, exp[0]); cl_assert_equal_i(3, exp[1]);
+		cl_assert_equal_i(3, exp[2]); cl_assert_equal_i(2, exp[3]);
+		break;
+	}
+
+	git_diff_free(diff);
+
+	return arg;
+}
+
+void test_threads_diff__concurrent_diffs(void)
+{
+	g_repo = cl_git_sandbox_init("status");
+
+	run_in_parallel(
+		20, 32, run_index_diffs, setup_trees, free_trees);
+}