thodg/libgit2

2012-09-10T21:29:07

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);

diff --git a/src/cache.c b/src/cache.c
index 3aa14f0..1f5b887 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;
 }