Commit 6ee68611 by Michael Schubert

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);
parent 2130dee4
......@@ -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;
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment