From 6ee6861123ccb599af584377dd8b75eeea24858b Mon Sep 17 00:00:00 2001 From: Michael Schubert Date: Mon, 10 Sep 2012 21:29:07 +0200 Subject: [PATCH] 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); --- src/cache.c | 9 +++++---- 1 file 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; }