Merge pull request #285 from Floweynt/async-nonblock

Do not pin threads when handling sync metrics

Author: sladkoff

src/main/java/de/sldk/mc/MetricRegistry.java

@@ -4,6 +4,7 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Objects;
 import java.util.concurrent.CompletableFuture;
 
 public class MetricRegistry {
@@ -28,6 +29,7 @@ CompletableFuture<Void> collectMetrics() {
         /* Combine all Completable futures into a single one */
         return CompletableFuture.allOf(this.metrics.stream()
                 .map(Metric::collect)
+                .filter(Objects::nonNull)
                 .toArray(CompletableFuture[]::new));
     }
 

src/main/java/de/sldk/mc/metrics/Metric.java

@@ -2,12 +2,11 @@
 
 import io.prometheus.client.Collector;
 import io.prometheus.client.CollectorRegistry;
-import org.bukkit.Bukkit;
-import org.bukkit.plugin.Plugin;
-
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ExecutionException;
 import java.util.logging.Logger;
+import org.bukkit.Bukkit;
+import org.bukkit.plugin.Plugin;
+import org.jetbrains.annotations.Nullable;
 
 public abstract class Metric {
 
@@ -27,42 +26,36 @@ protected Plugin getPlugin() {
         return plugin;
     }
 
+    @Nullable
     public CompletableFuture<Void> collect() {
-        return CompletableFuture.runAsync(() -> {
-            if (!enabled) {
-                return;
-            }
-
-            /* If metric is capable of async execution run it on a thread pool */
-            if (isAsyncCapable()) {
+        if (!isEnabled()) {
+            return null;
+        }
 
+        if (isAsyncCapable()) {
+            CompletableFuture.runAsync(() -> {
                 try {
                     doCollect();
-                }
-                catch (Exception e) {
+                } catch (Exception e) {
                     logException(e);
                 }
-                return;
-            }
+            });
+        }
 
-            /*
-            * Otherwise run the metric on the main thread and make the
-            * thread on thread pool wait for completion
-            */
+        CompletableFuture<Void> future = new CompletableFuture<>();
+
+        // don't call .get() - this blocks the ForkJoinPool.commonPool and may deadlock the server in some cases
+        Bukkit.getScheduler().callSyncMethod(plugin, () -> {
             try {
-                Bukkit.getScheduler().callSyncMethod(plugin, () -> {
-                    try {
-                        doCollect();
-                    }
-                    catch (Exception e) {
-                        logException(e);
-                    }
-                    return null;
-                }).get();
+                doCollect();
             } catch (Exception e) {
                 logException(e);
             }
+            future.complete(null);
+            return null;
         });
+
+        return future;
     }
 
     protected abstract void doCollect();