cacheLock and awaitLock

src/main/java/me/clip/placeholderapi/expansion/manager/CloudExpansionManager.java

@@ -46,6 +46,7 @@ import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.function.Function;
 import java.util.logging.Level;
 import java.util.stream.Collector;
@@ -76,8 +77,10 @@ public final class CloudExpansionManager {
 
   @NotNull
   private final Map<String, CloudExpansion> cache = new HashMap<>();
+  private final ReentrantLock cacheLock = new ReentrantLock();
   @NotNull
   private final Map<String, CompletableFuture<File>> await = new ConcurrentHashMap<>();
+  private final ReentrantLock awaitLock = new ReentrantLock();
 
   static final ExecutorService ASYNC_EXECUTOR =
       Executors.newCachedThreadPool(
@@ -113,12 +116,19 @@ public final class CloudExpansionManager {
   @NotNull
   @Unmodifiable
   public Map<String, CloudExpansion> getCloudExpansions() {
+    cacheLock.lock();
+    try {
       return ImmutableMap.copyOf(cache);
+    } finally {
+      cacheLock.unlock();
+    }
   }
 
   @NotNull
   @Unmodifiable
   public Map<String, CloudExpansion> getCloudExpansionsInstalled() {
+    cacheLock.lock();
+    try {
       if (cache.isEmpty()) {
         return Collections.emptyMap();
       }
@@ -127,11 +137,16 @@ public final class CloudExpansionManager {
           .stream()
           .filter(CloudExpansion::hasExpansion)
           .collect(INDEXED_NAME_COLLECTOR);
+    } finally {
+      cacheLock.unlock();
+    }
   }
 
   @NotNull
   @Unmodifiable
   public Map<String, CloudExpansion> getCloudExpansionsByAuthor(@NotNull final String author) {
+    cacheLock.lock();
+    try {
       if (cache.isEmpty()) {
         return Collections.emptyMap();
       }
@@ -140,12 +155,20 @@ public final class CloudExpansionManager {
           .stream()
           .filter(expansion -> author.equalsIgnoreCase(expansion.getAuthor()))
           .collect(INDEXED_NAME_COLLECTOR);
+    } finally {
+      cacheLock.unlock();
+    }
   }
 
   @NotNull
   @Unmodifiable
   public Set<String> getCloudExpansionAuthors() {
+    cacheLock.lock();
+    try {
       return cache.values().stream().map(CloudExpansion::getAuthor).collect(Collectors.toSet());
+    } finally {
+      cacheLock.unlock();
+    }
   }
 
   public int getCloudExpansionAuthorCount() {
@@ -163,14 +186,29 @@ public final class CloudExpansionManager {
 
   @NotNull
   public Optional<CloudExpansion> findCloudExpansionByName(@NotNull final String name) {
+    cacheLock.lock();
+    try {
       return Optional.ofNullable(cache.get(toIndexName(name)));
+    } finally {
+      cacheLock.unlock();
+    }
   }
 
   public void clean() {
+    cacheLock.lock();
+    try {
       cache.clear();
+    } finally {
+      cacheLock.unlock();
+    }
 
+    awaitLock.lock();
+    try {
       await.values().forEach(future -> future.cancel(true));
       await.clear();
+    } finally {
+      awaitLock.unlock();
+    }
   }
 
   public void fetch(final boolean allowUnverified) {
@@ -231,7 +269,12 @@ public final class CloudExpansionManager {
                           }
                         }
 
+                        cacheLock.lock();
+                        try {
                           cache.put(toIndexName(expansion), expansion);
+                        } finally {
+                          cacheLock.unlock();
+                        }
                       }
                     } catch (Throwable e) {
                       // ugly swallowing of every throwable, but we have to be defensive
@@ -244,13 +287,24 @@ public final class CloudExpansionManager {
   }
 
   public boolean isDownloading(@NotNull final CloudExpansion expansion) {
+    awaitLock.lock();
+    try {
       return await.containsKey(toIndexName(expansion));
+    } finally {
+      awaitLock.unlock();
+    }
   }
 
   @NotNull
   public CompletableFuture<File> downloadExpansion(@NotNull final CloudExpansion expansion,
       @NotNull final CloudExpansion.Version version) {
-    final CompletableFuture<File> previous = await.get(toIndexName(expansion));
+    CompletableFuture<File> previous;
+    awaitLock.lock();
+    try {
+      previous = await.get(toIndexName(expansion));
+    } finally {
+      awaitLock.unlock();
+    }
     if (previous != null) {
       return previous;
     }
@@ -269,7 +323,12 @@ public final class CloudExpansionManager {
     }, ASYNC_EXECUTOR);
 
     download.whenCompleteAsync((value, exception) -> {
+      awaitLock.lock();
+      try {
         await.remove(toIndexName(expansion));
+      } finally {
+        awaitLock.unlock();
+      }
 
       if (exception != null) {
         plugin.getLogger().log(Level.SEVERE,
@@ -277,7 +336,12 @@ public final class CloudExpansionManager {
       }
     }, ASYNC_EXECUTOR);
 
+    awaitLock.lock();
+    try {
       await.put(toIndexName(expansion), download);
+    } finally {
+      awaitLock.unlock();
+    }
 
     return download;
   }

src/main/java/me/clip/placeholderapi/util/Futures.java

@@ -71,7 +71,6 @@ public final class Futures {
   @NotNull
   private static <T> List<T> awaitCompletion(
       @NotNull final Collection<CompletableFuture<T>> futures) {
-    // TODO: Calling CompletableFuture#join is bad. Find a way to optimise this whole class
     return futures.stream().map(CompletableFuture::join).collect(Collectors.toList());
   }