Further threading fixes

This builds on the earlier thread safety work to make it so that
setting the odb, index, refdb, or config for a repository is done
in a threadsafe manner with minimized locking time.  This is done
by adding a lock to the repository object and using it to guard
the assignment of the above listed pointers.  The lock is only
held to assign the pointer value.

This also contains some minor fixes to the other work with pack
files to reduce the time that locks are being held to and fix an
apparently memory leak.

1 files changed, 131 insertions, 87 deletions

diff --git a/src/repository.c b/src/repository.c
index cda75b85f..8744b91e6 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -32,41 +32,43 @@
 
 #define GIT_TEMPLATE_DIR "/usr/share/git-core/templates"
 
+#define repo_swap_ptr(repo,item_ptr,new_value) \
+	git__swap(&repo->lock, (void **)item_ptr, new_value)
+
 static void drop_odb(git_repository *repo)
 {
-	if (repo->_odb != NULL) {
-		GIT_REFCOUNT_OWN(repo->_odb, NULL);
-		git_odb_free(repo->_odb);
-		repo->_odb = NULL;
+	git_odb *dropme = repo_swap_ptr(repo, &repo->_odb, NULL);
+	if (dropme != NULL) {
+		GIT_REFCOUNT_OWN(dropme, NULL);
+		git_odb_free(dropme);
 	}
 }
 
 static void drop_refdb(git_repository *repo)
 {
-	if (repo->_refdb != NULL) {
-		GIT_REFCOUNT_OWN(repo->_refdb, NULL);
-		git_refdb_free(repo->_refdb);
-		repo->_refdb = NULL;
+	git_refdb *dropme = repo_swap_ptr(repo, &repo->_refdb, NULL);
+	if (dropme != NULL) {
+		GIT_REFCOUNT_OWN(dropme, NULL);
+		git_refdb_free(dropme);
 	}
 }
 
 static void drop_config(git_repository *repo)
 {
-	if (repo->_config != NULL) {
-		GIT_REFCOUNT_OWN(repo->_config, NULL);
-		git_config_free(repo->_config);
-		repo->_config = NULL;
+	git_config *dropme = repo_swap_ptr(repo, &repo->_config, NULL);
+	if (dropme != NULL) {
+		GIT_REFCOUNT_OWN(dropme, NULL);
+		git_config_free(dropme);
+		git_repository__cvar_cache_clear(repo);
 	}
-
-	git_repository__cvar_cache_clear(repo);
 }
 
 static void drop_index(git_repository *repo)
 {
-	if (repo->_index != NULL) {
-		GIT_REFCOUNT_OWN(repo->_index, NULL);
-		git_index_free(repo->_index);
-		repo->_index = NULL;
+	git_index *dropme = repo_swap_ptr(repo, &repo->_index, NULL);
+	if (dropme != NULL) {
+		GIT_REFCOUNT_OWN(dropme, NULL);
+		git_index_free(dropme);
 	}
 }
 
@@ -79,14 +81,15 @@ void git_repository_free(git_repository *repo)
 	git_attr_cache_flush(repo);
 	git_submodule_config_free(repo);
 
-	git__free(repo->path_repository);
-	git__free(repo->workdir);
-
 	drop_config(repo);
 	drop_index(repo);
 	drop_odb(repo);
 	drop_refdb(repo);
 
+	git__free(repo->path_repository);
+	git__free(repo->workdir);
+
+	git_mutex_free(&repo->lock);
 	git__free(repo);
 }
 
@@ -119,6 +122,8 @@ static git_repository *repository_alloc(void)
 
 	memset(repo, 0x0, sizeof(git_repository));
 
+	git_mutex_init(&repo->lock);
+
 	if (git_cache_init(&repo->objects) < 0) {
 		git__free(repo);
 		return NULL;
@@ -549,39 +554,47 @@ on_error:
 	return error;
 }
 
-int git_repository_config__weakptr(git_config **out, git_repository *repo)
+static const char *path_unless_empty(git_buf *buf)
 {
-	if (repo->_config == NULL) {
-		git_buf global_buf = GIT_BUF_INIT, xdg_buf = GIT_BUF_INIT, system_buf = GIT_BUF_INIT;
-		int res;
-
-		const char *global_config_path = NULL;
-		const char *xdg_config_path = NULL;
-		const char *system_config_path = NULL;
-
-		if (git_config_find_global_r(&global_buf) == 0)
-			global_config_path = global_buf.ptr;
+	return git_buf_len(buf) > 0 ? git_buf_cstr(buf) : NULL;
+}
 
-		if (git_config_find_xdg_r(&xdg_buf) == 0)
-			xdg_config_path = xdg_buf.ptr;
+int git_repository_config__weakptr(git_config **out, git_repository *repo)
+{
+	int error = 0;
 
-		if (git_config_find_system_r(&system_buf) == 0)
-			system_config_path = system_buf.ptr;
+	if (repo->_config == NULL) {
+		git_buf global_buf = GIT_BUF_INIT;
+		git_buf xdg_buf = GIT_BUF_INIT;
+		git_buf system_buf = GIT_BUF_INIT;
+		git_config *config;
 
-		res = load_config(&repo->_config, repo, global_config_path, xdg_config_path, system_config_path);
+		git_config_find_global_r(&global_buf);
+		git_config_find_xdg_r(&xdg_buf);
+		git_config_find_system_r(&system_buf);
+
+		error = load_config(
+			&config, repo,
+			path_unless_empty(&global_buf),
+			path_unless_empty(&xdg_buf),
+			path_unless_empty(&system_buf));
+		if (!error) {
+			GIT_REFCOUNT_OWN(config, repo);
+
+			config = repo_swap_ptr(repo, &repo->_config, config);
+			if (config != NULL) {
+				GIT_REFCOUNT_OWN(config, NULL);
+				git_config_free(config);
+			}
+		}
 
 		git_buf_free(&global_buf);
 		git_buf_free(&xdg_buf);
 		git_buf_free(&system_buf);
-
-		if (res < 0)
-			return -1;
-
-		GIT_REFCOUNT_OWN(repo->_config, repo);
 	}
 
 	*out = repo->_config;
-	return 0;
+	return error;
 }
 
 int git_repository_config(git_config **out, git_repository *repo)
@@ -597,35 +610,46 @@ void git_repository_set_config(git_repository *repo, git_config *config)
 {
 	assert(repo && config);
 
-	drop_config(repo);
+	GIT_REFCOUNT_OWN(config, repo);
+	GIT_REFCOUNT_INC(config);
+
+	config = repo_swap_ptr(repo, &repo->_config, config);
+	if (config != NULL) {
+		GIT_REFCOUNT_OWN(config, NULL);
+		git_config_free(config);
+	}
 
-	repo->_config = config;
-	GIT_REFCOUNT_OWN(repo->_config, repo);
-	GIT_REFCOUNT_INC(repo->_config);
+	git_repository__cvar_cache_clear(repo);
 }
 
 int git_repository_odb__weakptr(git_odb **out, git_repository *repo)
 {
+	int error = 0;
+
 	assert(repo && out);
 
 	if (repo->_odb == NULL) {
 		git_buf odb_path = GIT_BUF_INIT;
-		int res;
+		git_odb *odb;
 
-		if (git_buf_joinpath(&odb_path, repo->path_repository, GIT_OBJECTS_DIR) < 0)
-			return -1;
+		git_buf_joinpath(&odb_path, repo->path_repository, GIT_OBJECTS_DIR);
 
-		res = git_odb_open(&repo->_odb, odb_path.ptr);
-		git_buf_free(&odb_path); /* done with path */
+		error = git_odb_open(&odb, odb_path.ptr);
+		if (!error) {
+			GIT_REFCOUNT_OWN(odb, repo);
 
-		if (res < 0)
-			return -1;
+			odb = repo_swap_ptr(repo, &repo->_odb, odb);
+			if (odb != NULL) {
+				GIT_REFCOUNT_OWN(odb, NULL);
+				git_odb_free(odb);
+			}
+		}
 
-		GIT_REFCOUNT_OWN(repo->_odb, repo);
+		git_buf_free(&odb_path);
 	}
 
 	*out = repo->_odb;
-	return 0;
+	return error;
 }
 
 int git_repository_odb(git_odb **out, git_repository *repo)
@@ -641,30 +665,39 @@ void git_repository_set_odb(git_repository *repo, git_odb *odb)
 {
 	assert(repo && odb);
 
-	drop_odb(repo);
-
-	repo->_odb = odb;
-	GIT_REFCOUNT_OWN(repo->_odb, repo);
+	GIT_REFCOUNT_OWN(odb, repo);
 	GIT_REFCOUNT_INC(odb);
+
+	odb = repo_swap_ptr(repo, &repo->_odb, odb);
+	if (odb != NULL) {
+		GIT_REFCOUNT_OWN(odb, NULL);
+		git_odb_free(odb);
+	}
 }
 
 int git_repository_refdb__weakptr(git_refdb **out, git_repository *repo)
 {
+	int error = 0;
+
 	assert(out && repo);
 
 	if (repo->_refdb == NULL) {
-		int res;
+		git_refdb *refdb;
 
-		res = git_refdb_open(&repo->_refdb, repo);
-
-		if (res < 0)
-			return -1;
+		error = git_refdb_open(&refdb, repo);
+		if (!error) {
+			GIT_REFCOUNT_OWN(refdb, repo);
 
-		GIT_REFCOUNT_OWN(repo->_refdb, repo);
+			refdb = repo_swap_ptr(repo, &repo->_refdb, refdb);
+			if (refdb != NULL) {
+				GIT_REFCOUNT_OWN(refdb, NULL);
+				git_refdb_free(refdb);
+			}
+		}
 	}
 
 	*out = repo->_refdb;
-	return 0;
+	return error;
 }
 
 int git_repository_refdb(git_refdb **out, git_repository *repo)
@@ -678,40 +711,48 @@ int git_repository_refdb(git_refdb **out, git_repository *repo)
 
 void git_repository_set_refdb(git_repository *repo, git_refdb *refdb)
 {
-	assert (repo && refdb);
+	assert(repo && refdb);
 
-	drop_refdb(repo);
-
-	repo->_refdb = refdb;
-	GIT_REFCOUNT_OWN(repo->_refdb, repo);
+	GIT_REFCOUNT_OWN(refdb, repo);
 	GIT_REFCOUNT_INC(refdb);
+
+	refdb = repo_swap_ptr(repo, &repo->_refdb, refdb);
+	if (refdb != NULL) {
+		GIT_REFCOUNT_OWN(refdb, NULL);
+		git_refdb_free(refdb);
+	}
 }
 
 int git_repository_index__weakptr(git_index **out, git_repository *repo)
 {
+	int error = 0;
+
 	assert(out && repo);
 
 	if (repo->_index == NULL) {
-		int res;
 		git_buf index_path = GIT_BUF_INIT;
+		git_index *index;
 
-		if (git_buf_joinpath(&index_path, repo->path_repository, GIT_INDEX_FILE) < 0)
-			return -1;
+		git_buf_joinpath(&index_path, repo->path_repository, GIT_INDEX_FILE);
 
-		res = git_index_open(&repo->_index, index_path.ptr);
-		git_buf_free(&index_path); /* done with path */
+		error = git_index_open(&index, index_path.ptr);
+		if (!error) {
+			GIT_REFCOUNT_OWN(index, repo);
 
-		if (res < 0)
-			return -1;
+			index = repo_swap_ptr(repo, &repo->_index, index);
+			if (index != NULL) {
+				GIT_REFCOUNT_OWN(index, NULL);
+				git_index_free(index);
+			}
 
-		GIT_REFCOUNT_OWN(repo->_index, repo);
+			error = git_index_set_caps(repo->_index, GIT_INDEXCAP_FROM_OWNER);
+		}
 
-		if (git_index_set_caps(repo->_index, GIT_INDEXCAP_FROM_OWNER) < 0)
-			return -1;
+		git_buf_free(&index_path);
 	}
 
 	*out = repo->_index;
-	return 0;
+	return error;
 }
 
 int git_repository_index(git_index **out, git_repository *repo)
@@ -727,11 +768,14 @@ void git_repository_set_index(git_repository *repo, git_index *index)
 {
 	assert(repo && index);
 
-	drop_index(repo);
-
-	repo->_index = index;
-	GIT_REFCOUNT_OWN(repo->_index, repo);
+	GIT_REFCOUNT_OWN(index, repo);
 	GIT_REFCOUNT_INC(index);
+
+	index = repo_swap_ptr(repo, &repo->_index, index);
+	if (index != NULL) {
+		GIT_REFCOUNT_OWN(index, NULL);
+		git_index_free(index);
+	}
 }
 
 static int check_repositoryformatversion(git_config *config)