cache: fix race condition

Example: a cached node is owned only by the cache (refcount == 1).
Thread A holds the lock and determines that the entry which should get
cached equals the node (git_oid_cmp(&node->oid, &entry->oid) == 0).
It frees the given entry to instead return the cached node to the user
(entry = node). Now, before Thread A happens to increment the refcount
of the node *outside* the cache lock, Thread B tries to store another
entry and hits the slot of the node before, decrements its refcount and
frees it *before* Thread A gets a chance to increment for the user.

	git_cached_obj_incref(entry);

	git_mutex_lock(&cache->lock);
	{
		git_cached_obj *node = cache->nodes[hash & cache->size_mask];

		if (node == NULL) {
			cache->nodes[hash & cache->size_mask] = entry;
		} else if (git_oid_cmp(&node->oid, &entry->oid) == 0) {
			git_cached_obj_decref(entry, cache->free_obj);
			entry = node;
		} else {
			git_cached_obj_decref(node, cache->free_obj);

// Thread B is here

			cache->nodes[hash & cache->size_mask] = entry;
		}
	}
	git_mutex_unlock(&cache->lock);

// Thread A is here

	/* increase the refcount again, because we are
	 * returning it to the user */
	git_cached_obj_incref(entry);

1 files changed, 5 insertions, 4 deletions

diff --git a/src/cache.c b/src/cache.c
index 3aa14f012..1f5b8872c 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -89,12 +89,13 @@ void *git_cache_try_store(git_cache *cache, void *_entry)
 			git_cached_obj_decref(node, cache->free_obj);
 			cache->nodes[hash & cache->size_mask] = entry;
 		}
+
+		/* increase the refcount again, because we are
+		 * returning it to the user */
+		git_cached_obj_incref(entry);
+
 	}
 	git_mutex_unlock(&cache->lock);
 
-	/* increase the refcount again, because we are
-	 * returning it to the user */
-	git_cached_obj_incref(entry);
-
 	return entry;
 }