Sort ref-names before locking them in filterAndLock

It is a common practise to avoid locking a set of resources in an
unpredictable order, because it would be subject to deadlocks.

Example of a possible deadlock is when two replication tasks are trying
to fetch two refs but in different order, e.g. refs/heads/branch1 and
refs/heads/branch2.

By locking the two refs in the same ordering, make sure that you never
have a situation whilst one task locks refs/heads/branch1 and the other
task locks refs/heads/branch2 and they both are stuck in trying each
other branches, when the task one tries to lock refs/heads/branch2 (lock
held by task two) and the task two tries to lock refs/heads/branch1
(lock held by task one).

Bug: Issue 437805590
Change-Id: I7fdb072e30e2313415c58e8c8dd39dd1a1f9f6e4
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultisiteReplicationFetchFilter.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultisiteReplicationFetchFilter.java
index ca79bcd..8a4503a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultisiteReplicationFetchFilter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/MultisiteReplicationFetchFilter.java
@@ -19,6 +19,7 @@
 import com.gerritforge.gerrit.globalrefdb.GlobalRefDbLockException;
 import com.gerritforge.gerrit.globalrefdb.RefDbLockException;
 import com.gerritforge.gerrit.globalrefdb.validation.SharedRefDatabaseWrapper;
+import com.google.common.collect.ImmutableSortedSet;
 import com.google.common.collect.Sets;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.entities.Project;
@@ -34,6 +35,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.SortedSet;
 import java.util.stream.Collectors;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
@@ -96,19 +98,23 @@
 
   @Override
   public Map<String, AutoCloseable> filterAndLock(String projectName, Set<String> fetchRefs) {
+    // Use a SortedSet to ensure that they are always locked in the same order and therefore
+    // prevents potential deadlocks where multiple fetch tasks are mutually locking each other
+    // by locking different refs in different orders.
+    SortedSet<String> sortedFetchRefs = ImmutableSortedSet.copyOf(fetchRefs);
     Project.NameKey projectKey = Project.nameKey(projectName);
     Set<String> filteredRefs = new HashSet<>();
     Map<String, AutoCloseable> refLocks = new HashMap<>();
     try {
-      for (String ref : fetchRefs) {
+      for (String ref : sortedFetchRefs) {
         refLocks.put(ref, sharedRefDb.lockLocalRef(projectKey, ref));
       }
-      filteredRefs.addAll(filter(projectName, fetchRefs));
+      filteredRefs.addAll(filter(projectName, sortedFetchRefs));
     } catch (RefDbLockException lockException) {
       filteredRefs.clear();
       throw lockException;
     } finally {
-      for (String excludedRef : Sets.difference(fetchRefs, filteredRefs)) {
+      for (String excludedRef : Sets.difference(sortedFetchRefs, filteredRefs)) {
         AutoCloseable excludedLock = refLocks.remove(excludedRef);
         if (excludedLock != null) {
           try {