From 9fa6264a1e0427c276af7819c84bbd547fa35b32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C5=A1a=20=C5=BDivkov?= Date: Fri, 2 Mar 2018 11:29:16 +0100 Subject: [PATCH] Project name locks: use Striped locks instead of Cache of locks Based on the review comments from Ben Manes in [1], using Cache of locks is prone to races. Using Striped locks technique is a better technique for this use case. [1] https://gerrit-review.googlesource.com/145750 Bug: Issue 7903 Change-Id: I94868904befc0f0454ae518f79888a8473ad97c2 --- .../DefaultProjectNameLockManager.java | 25 +++---------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/java/com/google/gerrit/server/project/DefaultProjectNameLockManager.java b/java/com/google/gerrit/server/project/DefaultProjectNameLockManager.java index 8c9bec73ca..3fb5d2af7c 100644 --- a/java/com/google/gerrit/server/project/DefaultProjectNameLockManager.java +++ b/java/com/google/gerrit/server/project/DefaultProjectNameLockManager.java @@ -14,17 +14,12 @@ package com.google.gerrit.server.project; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; +import com.google.common.util.concurrent.Striped; import com.google.gerrit.extensions.registration.DynamicItem; import com.google.gerrit.reviewdb.client.Project; import com.google.inject.AbstractModule; import com.google.inject.Singleton; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReentrantLock; @Singleton public class DefaultProjectNameLockManager implements ProjectNameLockManager { @@ -37,24 +32,10 @@ public class DefaultProjectNameLockManager implements ProjectNameLockManager { } } - LoadingCache lockCache = - CacheBuilder.newBuilder() - .maximumSize(1024) - .expireAfterAccess(5, TimeUnit.MINUTES) - .build( - new CacheLoader() { - @Override - public Lock load(Project.NameKey key) throws Exception { - return new ReentrantLock(); - } - }); + Striped locks = Striped.lock(10); @Override public Lock getLock(Project.NameKey name) { - try { - return lockCache.get(name); - } catch (ExecutionException e) { - throw new RuntimeException(e); - } + return locks.get(name); } }