Temporary solution to async-profiler JVM crashing issues (#271, #273, #274)

5 files changed, 179 insertions, 46 deletions

diff --git a/spark-common/src/main/java/me/lucko/spark/common/SparkPlatform.java b/spark-common/src/main/java/me/lucko/spark/common/SparkPlatform.java
index 2574443..dae04ff 100644
--- a/spark-common/src/main/java/me/lucko/spark/common/SparkPlatform.java
+++ b/spark-common/src/main/java/me/lucko/spark/common/SparkPlatform.java
@@ -44,12 +44,9 @@ import me.lucko.spark.common.monitor.net.NetworkMonitor;
 import me.lucko.spark.common.monitor.ping.PingStatistics;
 import me.lucko.spark.common.monitor.ping.PlayerPingProvider;
 import me.lucko.spark.common.monitor.tick.TickStatistics;
-import me.lucko.spark.common.platform.PlatformInfo;
 import me.lucko.spark.common.platform.PlatformStatisticsProvider;
-import me.lucko.spark.common.sampler.Sampler;
-import me.lucko.spark.common.sampler.SamplerBuilder;
+import me.lucko.spark.common.sampler.BackgroundSamplerManager;
 import me.lucko.spark.common.sampler.SamplerContainer;
-import me.lucko.spark.common.sampler.ThreadGrouper;
 import me.lucko.spark.common.sampler.source.ClassSourceLookup;
 import me.lucko.spark.common.tick.TickHook;
 import me.lucko.spark.common.tick.TickReporter;
@@ -104,6 +101,7 @@ public class SparkPlatform {
     private final ReentrantLock commandExecuteLock = new ReentrantLock(true);
     private final ActivityLog activityLog;
     private final SamplerContainer samplerContainer;
+    private final BackgroundSamplerManager backgroundSamplerManager;
     private final TickHook tickHook;
     private final TickReporter tickReporter;
     private final TickStatistics tickStatistics;
@@ -143,10 +141,8 @@ public class SparkPlatform {
         this.activityLog = new ActivityLog(plugin.getPluginDirectory().resolve("activity.json"));
         this.activityLog.load();
 
-        this.samplerContainer = new SamplerContainer(this.configuration.getBoolean(
-                "backgroundProfiler",
-                plugin.getPlatformInfo().getType() == PlatformInfo.Type.SERVER
-        ));
+        this.samplerContainer = new SamplerContainer();
+        this.backgroundSamplerManager = new BackgroundSamplerManager(this, this.configuration);
 
         this.tickHook = plugin.createTickHook();
         this.tickReporter = plugin.createTickReporter();
@@ -187,14 +183,7 @@ public class SparkPlatform {
         this.plugin.registerApi(api);
         SparkApi.register(api);
 
-        if (this.samplerContainer.isBackgroundProfilerEnabled()) {
-            this.plugin.log(Level.INFO, "Starting background profiler...");
-            try {
-                startBackgroundProfiler();
-            } catch (Throwable e) {
-                e.printStackTrace();
-            }
-        }
+        this.backgroundSamplerManager.initialise();
     }
 
     public void disable() {
@@ -255,6 +244,10 @@ public class SparkPlatform {
         return this.samplerContainer;
     }
 
+    public BackgroundSamplerManager getBackgroundSamplerManager() {
+        return this.backgroundSamplerManager;
+    }
+
     public TickHook getTickHook() {
         return this.tickHook;
     }
@@ -287,17 +280,6 @@ public class SparkPlatform {
         return this.serverNormalOperationStartTime;
     }
 
-    public void startBackgroundProfiler() {
-        Sampler sampler = new SamplerBuilder()
-                .background(true)
-                .threadDumper(this.plugin.getDefaultThreadDumper())
-                .threadGrouper(ThreadGrouper.BY_POOL)
-                .samplingInterval(this.configuration.getInteger("backgroundProfilerInterval", 10))
-                .start(this);
-
-        this.samplerContainer.setActiveSampler(sampler);
-    }
-
     public Path resolveSaveFile(String prefix, String extension) {
         Path pluginFolder = this.plugin.getPluginDirectory();
         try {
diff --git a/spark-common/src/main/java/me/lucko/spark/common/command/modules/SamplerModule.java b/spark-common/src/main/java/me/lucko/spark/common/command/modules/SamplerModule.java
index f576eac..cd00f0d 100644
--- a/spark-common/src/main/java/me/lucko/spark/common/command/modules/SamplerModule.java
+++ b/spark-common/src/main/java/me/lucko/spark/common/command/modules/SamplerModule.java
@@ -337,9 +337,7 @@ public class SamplerModule implements CommandModule {
             handleUpload(platform, resp, sampler, comment, mergeMode, saveToFile);
 
             // if the previous sampler was running in the background, create a new one
-            if (platform.getSamplerContainer().isBackgroundProfilerEnabled()) {
-                platform.startBackgroundProfiler();
-
+            if (platform.getBackgroundSamplerManager().restartBackgroundSampler()) {
                 resp.broadcastPrefixed(text()
                         .append(text("Restarted the background profiler. "))
                         .append(text("(If you don't want this to happen, run: /" + platform.getPlugin().getCommandName() + " profiler cancel)", DARK_GRAY))
diff --git a/spark-common/src/main/java/me/lucko/spark/common/sampler/BackgroundSamplerManager.java b/spark-common/src/main/java/me/lucko/spark/common/sampler/BackgroundSamplerManager.java
new file mode 100644
index 0000000..d655739
--- /dev/null
+++ b/spark-common/src/main/java/me/lucko/spark/common/sampler/BackgroundSamplerManager.java
@@ -0,0 +1,116 @@
+/*
+ * This file is part of spark.
+ *
+ *  Copyright (c) lucko (Luck) <luck@lucko.me>
+ *  Copyright (c) contributors
+ *
+ *  This program is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package me.lucko.spark.common.sampler;
+
+import me.lucko.spark.common.SparkPlatform;
+import me.lucko.spark.common.platform.PlatformInfo;
+import me.lucko.spark.common.util.Configuration;
+
+import java.util.logging.Level;
+
+public class BackgroundSamplerManager {
+
+    private static final String OPTION_ENABLED = "backgroundProfiler";
+    private static final String OPTION_ENGINE = "backgroundProfilerEngine";
+    private static final String OPTION_INTERVAL = "backgroundProfilerInterval";
+
+    private static final String MARKER_FAILED = "_marker_background_profiler_failed";
+
+    private final SparkPlatform platform;
+    private final Configuration configuration;
+    private final boolean enabled;
+
+    public BackgroundSamplerManager(SparkPlatform platform, Configuration configuration) {
+        this.platform = platform;
+        this.configuration = configuration;
+        this.enabled = this.configuration.getBoolean(
+                OPTION_ENABLED,
+                this.platform.getPlugin().getPlatformInfo().getType() == PlatformInfo.Type.SERVER
+        );
+    }
+
+    public void initialise() {
+        if (!this.enabled) {
+            return;
+        }
+
+        // are we enabling the background profiler by default for the first time?
+        boolean didEnableByDefault = false;
+        if (!this.configuration.contains(OPTION_ENABLED)) {
+            this.configuration.setBoolean(OPTION_ENABLED, true);
+            didEnableByDefault = true;
+        }
+
+        // did the background profiler fail to start on the previous attempt?
+        if (this.configuration.getBoolean(MARKER_FAILED, false)) {
+            this.platform.getPlugin().log(Level.WARNING, "It seems the background profiler failed to start when spark was last enabled. Sorry about that!");
+            this.platform.getPlugin().log(Level.WARNING, "In the future, spark will try to use the built-in Java profiling engine instead.");
+
+            this.configuration.remove(MARKER_FAILED);
+            this.configuration.setString(OPTION_ENGINE, "java");
+            this.configuration.save();
+        }
+
+        this.platform.getPlugin().log(Level.INFO, "Starting background profiler...");
+
+        if (didEnableByDefault) {
+            // set the failed marker and save before we try to start the profiler,
+            // then remove the marker afterwards if everything goes ok!
+            this.configuration.setBoolean(MARKER_FAILED, true);
+            this.configuration.save();
+        }
+
+        try {
+            startSampler();
+
+            if (didEnableByDefault) {
+                this.configuration.remove(MARKER_FAILED);
+                this.configuration.save();
+            }
+
+        } catch (Throwable e) {
+            e.printStackTrace();
+        }
+    }
+
+    public boolean restartBackgroundSampler() {
+        if (this.enabled) {
+            startSampler();
+            return true;
+        }
+        return false;
+    }
+
+    private void startSampler() {
+        boolean forceJavaEngine = this.configuration.getString(OPTION_ENGINE, "async").equals("java");
+
+        Sampler sampler = new SamplerBuilder()
+                .background(true)
+                .threadDumper(this.platform.getPlugin().getDefaultThreadDumper())
+                .threadGrouper(ThreadGrouper.BY_POOL)
+                .samplingInterval(this.configuration.getInteger(OPTION_INTERVAL, 10))
+                .forceJavaSampler(forceJavaEngine)
+                .start(this.platform);
+
+        this.platform.getSamplerContainer().setActiveSampler(sampler);
+    }
+
+}
diff --git a/spark-common/src/main/java/me/lucko/spark/common/sampler/SamplerContainer.java b/spark-common/src/main/java/me/lucko/spark/common/sampler/SamplerContainer.java
index d55909c..15b1029 100644
--- a/spark-common/src/main/java/me/lucko/spark/common/sampler/SamplerContainer.java
+++ b/spark-common/src/main/java/me/lucko/spark/common/sampler/SamplerContainer.java
@@ -28,11 +28,6 @@ import java.util.concurrent.atomic.AtomicReference;
 public class SamplerContainer implements AutoCloseable {
 
     private final AtomicReference<Sampler> activeSampler = new AtomicReference<>();
-    private final boolean backgroundProfilerEnabled;
-
-    public SamplerContainer(boolean backgroundProfilerEnabled) {
-        this.backgroundProfilerEnabled = backgroundProfilerEnabled;
-    }
 
     /**
      * Gets the active sampler, or null if a sampler is not active.
@@ -73,10 +68,6 @@ public class SamplerContainer implements AutoCloseable {
         }
     }
 
-    public boolean isBackgroundProfilerEnabled() {
-        return this.backgroundProfilerEnabled;
-    }
-
     @Override
     public void close() {
         stopActiveSampler(true);
diff --git a/spark-common/src/main/java/me/lucko/spark/common/util/Configuration.java b/spark-common/src/main/java/me/lucko/spark/common/util/Configuration.java
index ce63878..32f3bc6 100644
--- a/spark-common/src/main/java/me/lucko/spark/common/util/Configuration.java
+++ b/spark-common/src/main/java/me/lucko/spark/common/util/Configuration.java
@@ -20,32 +20,58 @@
 
 package me.lucko.spark.common.util;
 
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
 import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
-import com.google.gson.JsonParser;
 import com.google.gson.JsonPrimitive;
 
 import java.io.BufferedReader;
+import java.io.BufferedWriter;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 
 public final class Configuration {
-    private static final JsonParser PARSER = new JsonParser();
+    private static final Gson GSON = new GsonBuilder().setPrettyPrinting().create();
 
-    private final JsonObject root;
+    private final Path file;
+    private JsonObject root;
 
     public Configuration(Path file) {
+        this.file = file;
+        load();
+    }
+
+    public void load() {
         JsonObject root = null;
-        if (Files.exists(file)) {
-            try (BufferedReader reader = Files.newBufferedReader(file, StandardCharsets.UTF_8)) {
-                root = PARSER.parse(reader).getAsJsonObject();
+        if (Files.exists(this.file)) {
+            try (BufferedReader reader = Files.newBufferedReader(this.file, StandardCharsets.UTF_8)) {
+                root = GSON.fromJson(reader, JsonObject.class);
             } catch (IOException e) {
                 e.printStackTrace();
             }
         }
-        this.root = root != null ? root : new JsonObject();
+        if (root == null) {
+            root = new JsonObject();
+            root.addProperty("_header", "spark configuration file - https://spark.lucko.me/docs/Configuration");
+        }
+        this.root = root;
+    }
+
+    public void save() {
+        try {
+            Files.createDirectories(this.file.getParent());
+        } catch (IOException e) {
+            // ignore
+        }
+
+        try (BufferedWriter writer = Files.newBufferedWriter(this.file, StandardCharsets.UTF_8)) {
+            GSON.toJson(this.root, writer);
+        } catch (IOException e) {
+            e.printStackTrace();
+        }
     }
 
     public String getString(String path, String def) {
@@ -77,4 +103,24 @@ public final class Configuration {
         return val.isBoolean() ? val.getAsInt() : def;
     }
 
+    public void setString(String path, String value) {
+        this.root.add(path, new JsonPrimitive(value));
+    }
+
+    public void setBoolean(String path, boolean value) {
+        this.root.add(path, new JsonPrimitive(value));
+    }
+
+    public void setInteger(String path, int value) {
+        this.root.add(path, new JsonPrimitive(value));
+    }
+
+    public boolean contains(String path) {
+        return this.root.has(path);
+    }
+
+    public void remove(String path) {
+        this.root.remove(path);
+    }
+
 }