Start leaderboard cache only after PAPI registration and stop refreshes cleanly on disable

Author: nossr50

src/main/java/com/gmail/nossr50/mcMMO.java

@@ -393,7 +393,14 @@ public void onEnable() {
         if (Bukkit.getPluginManager().getPlugin("PlaceholderAPI") != null) {
             // Keep a reference for explicit shutdown/unregister in onDisable().
             papiExpansion = new PapiExpansion();
-            papiExpansion.register();
+            if (papiExpansion.register()) {
+                // Only spend refresh work on the leaderboard cache once PlaceholderAPI has
+                // actually accepted the expansion.
+                papiExpansion.startLeaderboardCache();
+            } else {
+                getLogger().warning(
+                        "Failed to register the mcMMO PlaceholderAPI expansion, mcMMO placeholders will be unavailable.");
+            }
         }
     }
 

src/main/java/com/gmail/nossr50/placeholders/LeaderboardPlaceholderCache.java

@@ -39,6 +39,9 @@ public class LeaderboardPlaceholderCache {
     // Single-flight guard to avoid overlapping rebuild work.
     private final AtomicBoolean refreshInProgress = new AtomicBoolean(false);
     private volatile @Nullable WrappedTask refreshTask;
+    // Once stopped, late-firing refreshes become no-ops so they cannot race database shutdown
+    // during plugin disable.
+    private volatile boolean stopped;
 
     /**
      * Constructor.
@@ -112,9 +115,11 @@ private boolean isDatabaseReady() {
     }
 
     /**
-     * Stops the periodic refresh task if active.
+     * Stops the periodic refresh task if active and turns any further refresh attempts into
+     * no-ops.
      */
     public void shutdown() {
+        stopped = true;
         final WrappedTask localTask = refreshTask;
         if (localTask != null) {
             localTask.cancel();
@@ -152,6 +157,11 @@ public String getValue(@Nullable PrimarySkillType skill, int position) {
      * was skipped (already running) or failed.
      */
     boolean refreshNow() {
+        // A stopped cache never refreshes; its snapshot only serves until the plugin disables.
+        if (stopped) {
+            return false;
+        }
+
         // Runtime guard: never execute refresh logic on the primary thread.
         if (plugin != null && Bukkit.isPrimaryThread()) {
             plugin.getFoliaLib().getScheduler().runAsync(task -> refreshNow());
@@ -171,7 +181,10 @@ boolean refreshNow() {
             snapshot.set(buildSnapshot());
             return true;
         } catch (RuntimeException e) {
-            logger.log(Level.WARNING, "Failed to refresh PlaceholderAPI leaderboard cache", e);
+            // A refresh interrupted by plugin disable is expected; don't warn about it.
+            if (!stopped) {
+                logger.log(Level.WARNING, "Failed to refresh PlaceholderAPI leaderboard cache", e);
+            }
             return false;
         } finally {
             refreshInProgress.set(false);

src/main/java/com/gmail/nossr50/placeholders/PapiExpansion.java

@@ -43,6 +43,14 @@ public PapiExpansion() {
                 refreshIntervalTicks
         );
         init();
+    }
+
+    /**
+     * Starts the leaderboard cache's async warm-up and periodic refreshes. Call only after this
+     * expansion registered successfully, so no refresh work runs for an expansion that
+     * PlaceholderAPI rejected.
+     */
+    public void startLeaderboardCache() {
         leaderboardPlaceholderCache.start();
     }
 

src/test/java/com/gmail/nossr50/placeholders/LeaderboardPlaceholderCacheTest.java

@@ -114,6 +114,33 @@ void shouldKeepLastGoodSnapshotWhenRefreshFails() {
         assertThat(cache.getValue(null, 1)).isEqualTo("1000");
     }
 
+    /**
+     * A cache that has been shut down must stop touching its data source entirely: a refresh
+     * firing during plugin disable would otherwise race database shutdown.
+     */
+    @Test
+    void refreshNowShouldNotReadDataSourceWhenCacheIsShutDown() {
+        // Given - a cache with one good snapshot that is then shut down.
+        RecordingDataSource dataSource = new RecordingDataSource();
+        dataSource.overall = List.of(new PlayerStat("survivor", 1000));
+
+        LeaderboardPlaceholderCache cache = new LeaderboardPlaceholderCache(5, dataSource,
+                Logger.getAnonymousLogger());
+        assertThat(cache.refreshNow()).isTrue();
+        int readsBeforeShutdown = dataSource.calls.size();
+
+        // When - shutting down and attempting another refresh.
+        cache.shutdown();
+        boolean refreshed = cache.refreshNow();
+
+        // Then - the refresh is rejected and no further data source reads happen.
+        assertThat(refreshed).isFalse();
+        assertThat(dataSource.calls).hasSize(readsBeforeShutdown);
+
+        // And - the existing snapshot still serves lookups until the plugin fully disables.
+        assertThat(cache.getPlayerName(null, 1)).isEqualTo("survivor");
+    }
+
     private static final class RecordingDataSource implements
             LeaderboardPlaceholderCache.LeaderboardDataSource {
         private final List<DataSourceCall> calls = new CopyOnWriteArrayList<>();