Merge pull request #1127 from libgit2/fix-mwindow-threading

Fix mwindow mutex initialization and error checking

5 files changed, 54 insertions, 15 deletions

diff --git a/src/cache.c b/src/cache.c
index edd3a47dd..cbd360a02 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -52,7 +52,11 @@ void *git_cache_get(git_cache *cache, const git_oid *oid)
 
 	memcpy(&hash, oid->id, sizeof(hash));
 
-	git_mutex_lock(&cache->lock);
+	if (git_mutex_lock(&cache->lock)) {
+		giterr_set(GITERR_THREAD, "unable to lock cache mutex");
+		return NULL;
+	}
+
 	{
 		node = cache->nodes[hash & cache->size_mask];
 
@@ -73,14 +77,18 @@ void *git_cache_try_store(git_cache *cache, void *_entry)
 
 	memcpy(&hash, &entry->oid, sizeof(uint32_t));
 
-	/* increase the refcount on this object, because
-	 * the cache now owns it */
-	git_cached_obj_incref(entry);
+	if (git_mutex_lock(&cache->lock)) {
+		giterr_set(GITERR_THREAD, "unable to lock cache mutex");
+		return NULL;
+	}
 
-	git_mutex_lock(&cache->lock);
 	{
 		git_cached_obj *node = cache->nodes[hash & cache->size_mask];
 
+		/* increase the refcount on this object, because
+		 * the cache now owns it */
+		git_cached_obj_incref(entry);
+
 		if (node == NULL) {
 			cache->nodes[hash & cache->size_mask] = entry;
 		} else if (git_oid_cmp(&node->oid, &entry->oid) == 0) {
diff --git a/src/global.c b/src/global.c
index d085089c3..305ec2edd 100644
--- a/src/global.c
+++ b/src/global.c
@@ -119,6 +119,7 @@ int git_threads_init(void)
 	if (_tls_init)
 		return 0;
 
+	git_mutex_init(&git__mwindow_mutex);
 	pthread_key_create(&_tls_key, &cb__free_status);
 
 	/* Initialize any other subsystems that have global state */
@@ -134,6 +135,7 @@ void git_threads_shutdown(void)
 {
 	pthread_key_delete(_tls_key);
 	_tls_init = 0;
+	git_mutex_free(&git__mwindow_mutex);
 
 	/* Shut down any subsystems that have global state */
 	git_hash_global_shutdown();
diff --git a/src/mwindow.c b/src/mwindow.c
index 4da5badb6..ee693e4a0 100644
--- a/src/mwindow.c
+++ b/src/mwindow.c
@@ -44,7 +44,10 @@ void git_mwindow_free_all(git_mwindow_file *mwf)
 	git_mwindow_ctl *ctl = &mem_ctl;
 	unsigned int i;
 
-	git_mutex_lock(&git__mwindow_mutex);
+	if (git_mutex_lock(&git__mwindow_mutex)) {
+		giterr_set(GITERR_THREAD, "unable to lock mwindow mutex");
+		return;
+	}
 
 	/*
 	 * Remove these windows from the global list
@@ -221,7 +224,11 @@ unsigned char *git_mwindow_open(
 	git_mwindow_ctl *ctl = &mem_ctl;
 	git_mwindow *w = *cursor;
 
-	git_mutex_lock(&git__mwindow_mutex);
+	if (git_mutex_lock(&git__mwindow_mutex)) {
+		giterr_set(GITERR_THREAD, "unable to lock mwindow mutex");
+		return NULL;
+	}
+
 	if (!w || !(git_mwindow_contains(w, offset) && git_mwindow_contains(w, offset + extra))) {
 		if (w) {
 			w->inuse_cnt--;
@@ -269,7 +276,11 @@ int git_mwindow_file_register(git_mwindow_file *mwf)
 	git_mwindow_ctl *ctl = &mem_ctl;
 	int ret;
 
-	git_mutex_lock(&git__mwindow_mutex);
+	if (git_mutex_lock(&git__mwindow_mutex)) {
+		giterr_set(GITERR_THREAD, "unable to lock mwindow mutex");
+		return -1;
+	}
+
 	if (ctl->windowfiles.length == 0 &&
 	    git_vector_init(&ctl->windowfiles, 8, NULL) < 0) {
 		git_mutex_unlock(&git__mwindow_mutex);
@@ -288,7 +299,11 @@ int git_mwindow_file_deregister(git_mwindow_file *mwf)
 	git_mwindow_file *cur;
 	unsigned int i;
 
-	git_mutex_lock(&git__mwindow_mutex);
+	if (git_mutex_lock(&git__mwindow_mutex)) {
+		giterr_set(GITERR_THREAD, "unable to lock mwindow mutex");
+		return -1;
+	}
+
 	git_vector_foreach(&ctl->windowfiles, i, cur) {
 		if (cur == mwf) {
 			git_vector_remove(&ctl->windowfiles, i);
@@ -306,7 +321,11 @@ void git_mwindow_close(git_mwindow **window)
 {
 	git_mwindow *w = *window;
 	if (w) {
-		git_mutex_lock(&git__mwindow_mutex);
+		if (git_mutex_lock(&git__mwindow_mutex)) {
+			giterr_set(GITERR_THREAD, "unable to lock mwindow mutex");
+			return;
+		}
+
 		w->inuse_cnt--;
 		git_mutex_unlock(&git__mwindow_mutex);
 		*window = NULL;
diff --git a/src/pack-objects.c b/src/pack-objects.c
index 44ad3fd98..2eb69764d 100644
--- a/src/pack-objects.c
+++ b/src/pack-objects.c
@@ -1025,6 +1025,14 @@ static void *threaded_find_deltas(void *arg)
 		git_cond_signal(&me->pb->progress_cond);
 		git_packbuilder__progress_unlock(me->pb);
 
+		if (git_mutex_lock(&me->mutex)) {
+			giterr_set(GITERR_THREAD, "unable to lock packfile condition mutex");
+			return NULL;
+		}
+
+		while (!me->data_ready)
+			git_cond_wait(&me->cond, &me->mutex);
+
 		/*
 		 * We must not set ->data_ready before we wait on the
 		 * condition because the main thread may have set it to 1
@@ -1033,9 +1041,6 @@ static void *threaded_find_deltas(void *arg)
 		 * was initialized to 0 before this thread was spawned
 		 * and we reset it to 0 right away.
 		 */
-		git_mutex_lock(&me->mutex);
-		while (!me->data_ready)
-			git_cond_wait(&me->cond, &me->mutex);
 		me->data_ready = 0;
 		git_mutex_unlock(&me->mutex);
 	}
@@ -1168,7 +1173,12 @@ static int ll_find_deltas(git_packbuilder *pb, git_pobject **list,
 		target->working = 1;
 		git_packbuilder__progress_unlock(pb);
 
-		git_mutex_lock(&target->mutex);
+		if (git_mutex_lock(&target->mutex)) {
+			giterr_set(GITERR_THREAD, "unable to lock packfile condition mutex");
+			git__free(p);
+			return -1;
+		}
+
 		target->data_ready = 1;
 		git_cond_signal(&target->cond);
 		git_mutex_unlock(&target->mutex);
diff --git a/src/thread-utils.h b/src/thread-utils.h
index f0a51f28c..d747a0d30 100644
--- a/src/thread-utils.h
+++ b/src/thread-utils.h
@@ -79,7 +79,7 @@ GIT_INLINE(int) git_atomic_dec(git_atomic *a)
 /* Pthreads Mutex */
 #define git_mutex unsigned int
 #define git_mutex_init(a) (void)0
-#define git_mutex_lock(a) (void)0
+#define git_mutex_lock(a) 0
 #define git_mutex_unlock(a) (void)0
 #define git_mutex_free(a) (void)0