switch to ReentrantLock: this will provide ability to use tryLock
authorPavel Sher <pavel.sher@jetbrains.com>
Mon, 28 Jan 2019 17:08:41 +0000 (18:08 +0100)
committerPavel Sher <pavel.sher@jetbrains.com>
Mon, 28 Jan 2019 17:08:41 +0000 (18:08 +0100)
git-server/src/jetbrains/buildServer/buildTriggers/vcs/git/Cleanup.java
git-server/src/jetbrains/buildServer/buildTriggers/vcs/git/CommitLoader.java
git-server/src/jetbrains/buildServer/buildTriggers/vcs/git/CommitLoaderImpl.java
git-server/src/jetbrains/buildServer/buildTriggers/vcs/git/GitCommitSupport.java
git-server/src/jetbrains/buildServer/buildTriggers/vcs/git/GitMergeSupport.java
git-server/src/jetbrains/buildServer/buildTriggers/vcs/git/RepositoryManager.java
git-server/src/jetbrains/buildServer/buildTriggers/vcs/git/RepositoryManagerImpl.java

index 4d05289f179004c0c5dafb61c92de7ea9e51f940..76c82eea0f84163b7814ba1f52c320609ae89ae0 100644 (file)
@@ -42,6 +42,7 @@ import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.function.Consumer;
 import java.util.regex.Pattern;
 
@@ -183,8 +184,12 @@ public class Cleanup {
       }
       if (enoughDiskSpaceForGC(gitDir, freeDiskSpace)) {
         if (runInPlace) {
-          synchronized (myRepositoryManager.getWriteLock(gitDir)) {
+          ReentrantLock lock = myRepositoryManager.getWriteLock(gitDir);
+          lock.lock();
+          try {
             runNativeGC(gitDir);
+          } finally {
+            lock.unlock();
           }
         } else {
           runGcInCopy(gitDir);
@@ -475,7 +480,9 @@ public class Cleanup {
     Boolean nativeGitInstalled = null;
     boolean enableNativeGitLogged = false;
     for (File gitDir : allDirs) {
-      synchronized (myRepositoryManager.getWriteLock(gitDir)) {
+      ReentrantLock lock = myRepositoryManager.getWriteLock(gitDir);
+      lock.lock();
+      try {
         try {
           LOG.info("Start garbage collection in " + gitDir.getAbsolutePath());
           long repositoryStartNanos = System.nanoTime();
@@ -499,7 +506,10 @@ public class Cleanup {
             }
           }
         }
+      } finally {
+        lock.unlock();
       }
+
       runGCCounter++;
       final long repositoryFinishNanos = System.nanoTime();
       if ((repositoryFinishNanos - startNanos) > gcTimeQuotaNanos) {
index d1ec7ee152f8682cbef07c2b04e4e272209a172d..22817be8aa62b22f0f4691c7392ffe8094aa7df6 100644 (file)
@@ -38,7 +38,7 @@ public interface CommitLoader {
                        @NotNull GitVcsRoot root,
                        @NotNull String revision) throws VcsException, IOException;
 
-  public void fetch(@NotNull Repository db,
+  void fetch(@NotNull Repository db,
                     @NotNull URIish fetchURI,
                     @NotNull Collection<RefSpec> refspecs,
                     @NotNull FetchSettings settings) throws IOException, VcsException;
@@ -47,5 +47,5 @@ public interface CommitLoader {
   RevCommit getCommit(@NotNull Repository repository, @NotNull ObjectId commitId) throws IOException;
 
   @Nullable
-  public RevCommit findCommit(@NotNull Repository r, @NotNull String sha);
+  RevCommit findCommit(@NotNull Repository r, @NotNull String sha);
 }
index 330de7fb7c8467a1b96b6a38f1abd29ddb817df0..637cb3aa8dd9db8c3b6cf6c8744ce6292329b803 100644 (file)
@@ -33,13 +33,14 @@ import java.io.IOException;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.locks.ReentrantLock;
 
 import static java.util.Arrays.asList;
 
 public class CommitLoaderImpl implements CommitLoader {
 
   private static final Logger LOG = Logger.getInstance(CommitLoaderImpl.class.getName());
-  private static final Logger PERFORMANCE_LOG = Logger.getInstance(CommitLoaderImpl.class.getName() + ".Performance");
+  public static final Logger PERFORMANCE_LOG = Logger.getInstance(CommitLoaderImpl.class.getName() + ".Performance");
 
   private final RepositoryManager myRepositoryManager;
   private final FetchCommand myFetchCommand;
@@ -92,7 +93,10 @@ public class CommitLoaderImpl implements CommitLoader {
     File repositoryDir = db.getDirectory();
     assert repositoryDir != null : "Non-local repository";
     final long start = System.currentTimeMillis();
-    synchronized (myRepositoryManager.getWriteLock(repositoryDir)) {
+
+    ReentrantLock lock = myRepositoryManager.getWriteLock(repositoryDir);
+    lock.lock();
+    try {
       final long finish = System.currentTimeMillis();
       final long waitTime = finish - start;
       if (waitTime > 20000) {
@@ -104,6 +108,8 @@ public class CommitLoaderImpl implements CommitLoader {
       myFetchCommand.fetch(db, fetchURI, refspecs, settings);
       Map<String, Ref> newRefs = new HashMap<>(db.getAllRefs());
       myMapFullPath.invalidateRevisionsCache(db, oldRefs, newRefs);
+    } finally {
+      lock.unlock();
     }
   }
 
index a328cf9b81edafb62eb053b7d6aaa4f74cccfc90..08e405fabb5a9ec969c17a9e2fa21c139159dcb9 100644 (file)
@@ -40,6 +40,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.util.*;
 import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 
 import static java.util.Arrays.asList;
 
@@ -163,7 +164,9 @@ public class GitCommitSupport implements CommitSupport, GitServerExtension {
 
         ObjectId commitId = createCommit(gitRoot, lastCommit, treeId, commitSettings.getUserName(), nonEmptyMessage(commitSettings));
 
-        synchronized (myRepositoryManager.getWriteLock(gitRoot.getRepositoryDir())) {
+        ReentrantLock lock = myRepositoryManager.getWriteLock(gitRoot.getRepositoryDir());
+        lock.lock();
+        try {
           final Transport tn = myTransportFactory.createTransport(myDb, gitRoot.getRepositoryPushURL(), gitRoot.getAuthSettings(),
                                                                   myPluginConfig.getPushTimeoutSeconds());
           try {
@@ -188,6 +191,8 @@ public class GitCommitSupport implements CommitSupport, GitServerExtension {
           } finally {
             tn.close();
           }
+        } finally {
+          lock.unlock();
         }
       } catch (Exception e) {
         throw myContext.wrapException(e);
index d9fcf937d4861a63e521a91171e7088954aadd7b..b59646cc047be6cc4c646167f60934089a86c462 100644 (file)
@@ -34,6 +34,7 @@ import org.jetbrains.annotations.NotNull;
 
 import java.io.IOException;
 import java.util.*;
+import java.util.concurrent.locks.ReentrantLock;
 
 import static java.util.Arrays.asList;
 
@@ -159,7 +160,9 @@ public class GitMergeSupport implements MergeSupport, GitServerExtension {
       return MergeResult.createMergeError(e.getConflicts());
     }
 
-    synchronized (myRepositoryManager.getWriteLock(gitRoot.getRepositoryDir())) {
+    ReentrantLock lock = myRepositoryManager.getWriteLock(gitRoot.getRepositoryDir());
+    lock.lock();
+    try {
       final Transport tn = myTransportFactory.createTransport(db, gitRoot.getRepositoryPushURL(), gitRoot.getAuthSettings(),
                                                               myPluginConfig.getPushTimeoutSeconds());
       try {
@@ -178,6 +181,8 @@ public class GitMergeSupport implements MergeSupport, GitServerExtension {
       } finally {
         tn.close();
       }
+    } finally {
+      lock.unlock();
     }
   }
 
index 7c7576b1a46e35d90b6d46ed90c356ea36d2d943..a5008bf1313dee2312a2261b023cef6286961376 100644 (file)
@@ -24,6 +24,7 @@ import org.jetbrains.annotations.NotNull;
 import java.io.File;
 import java.util.List;
 import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantLock;
 
 /**
  * @author dmitry.neverov
@@ -42,7 +43,7 @@ public interface RepositoryManager extends MirrorManager {
   void closeRepository(@NotNull Repository repository);
 
   @NotNull
-  Object getWriteLock(@NotNull File dir);
+  ReentrantLock getWriteLock(@NotNull File dir);
 
   @NotNull
   ReadWriteLock getRmLock(@NotNull File dir);
index 5009224dc01c31b7b9e7853f17e0101fb872d06d..e15b990fc07ee5c6802011f4bdc62abafc65b8c7 100644 (file)
@@ -36,6 +36,7 @@ import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import static jetbrains.buildServer.buildTriggers.vcs.git.GitServerUtil.getWrongUrlError;
@@ -60,7 +61,7 @@ public final class RepositoryManagerImpl implements RepositoryManager {
    * Also several concurrent fetches in single repository does not make sense since only one of them succeed.
    * This map contains locks used for fetch and push operations.
    */
-  private final ConcurrentMap<String, Object> myWriteLocks = new ConcurrentHashMap<>();
+  private final ConcurrentMap<String, ReentrantLock> myWriteLocks = new ConcurrentHashMap<>();
   /**
    * During cleanup unused bare repositories are removed. This map contains rw locks for repository removal.
    * Fetch/push/create operations should be done with read lock hold, remove operation is done with write lock hold.
@@ -225,8 +226,8 @@ public final class RepositoryManagerImpl implements RepositoryManager {
 
 
   @NotNull
-  public Object getWriteLock(@NotNull final File dir) {
-    return getOrCreate(myWriteLocks, getCanonicalName(dir), new Object());
+  public ReentrantLock getWriteLock(@NotNull final File dir) {
+    return getOrCreate(myWriteLocks, getCanonicalName(dir), new ReentrantLock());
   }