From 55cf34508a045b1d3239c0af3cc0cd6060218e9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20S=C3=B6derberg?= Date: Tue, 23 Jun 2020 18:36:53 +0200 Subject: [PATCH] Fix issue where old player objects were not cleaned up properly. This is caused by an issue with the event order in Spigot (and Spigot derivatives), so the fix is rather hacky. --- .../com/plotsquared/bukkit/BukkitMain.java | 15 ++++++++++- .../bukkit/listener/PlayerEvents.java | 4 +-- .../plotsquared/bukkit/util/BukkitUtil.java | 4 ++- .../java/com/plotsquared/core/plot/Plot.java | 25 ++++++++++--------- .../plotsquared/core/util/PlayerManager.java | 16 ++++++++++-- 5 files changed, 46 insertions(+), 18 deletions(-) diff --git a/Bukkit/src/main/java/com/plotsquared/bukkit/BukkitMain.java b/Bukkit/src/main/java/com/plotsquared/bukkit/BukkitMain.java index c423d8102..1a40c51d5 100644 --- a/Bukkit/src/main/java/com/plotsquared/bukkit/BukkitMain.java +++ b/Bukkit/src/main/java/com/plotsquared/bukkit/BukkitMain.java @@ -392,6 +392,19 @@ public final class BukkitMain extends JavaPlugin implements Listener, IPlotMain< PlotSquared.log( Captions.PREFIX.getTranslated() + "Using platform world manager: " + this.worldManager .getName()); + + // Clean up potential memory leak + Bukkit.getScheduler().runTaskTimer(this, () -> { + try { + for (final PlotPlayer player : this.getPlayerManager().getPlayers()) { + if (player.getPlatformPlayer() == null || !player.getPlatformPlayer().isOnline()) { + this.getPlayerManager().removePlayer(player); + } + } + } catch (final Exception e) { + getLogger().warning("Failed to clean up players: " + e.getMessage()); + } + }, 100L, 100L); } private void unload() { @@ -982,7 +995,7 @@ public final class BukkitMain extends JavaPlugin implements Listener, IPlotMain< } @Override public void unregister(@NonNull final PlotPlayer player) { - BukkitUtil.removePlayer(player.getName()); + BukkitUtil.removePlayer(player.getUUID()); } @Override public void registerChunkProcessor() { diff --git a/Bukkit/src/main/java/com/plotsquared/bukkit/listener/PlayerEvents.java b/Bukkit/src/main/java/com/plotsquared/bukkit/listener/PlayerEvents.java index f94c54638..643f80611 100644 --- a/Bukkit/src/main/java/com/plotsquared/bukkit/listener/PlayerEvents.java +++ b/Bukkit/src/main/java/com/plotsquared/bukkit/listener/PlayerEvents.java @@ -650,10 +650,10 @@ public class PlayerEvents extends PlotListener implements Listener { PlotSquared.get().getImpromptuUUIDPipeline().storeImmediately(event.getName(), uuid); } - @EventHandler(priority = EventPriority.LOWEST, ignoreCancelled = true) + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true) public void onConnect(PlayerJoinEvent event) { final Player player = event.getPlayer(); - BukkitUtil.removePlayer(player.getName()); + BukkitUtil.removePlayer(player.getUniqueId()); final PlotPlayer pp = BukkitUtil.getPlayer(player); Location location = pp.getLocation(); diff --git a/Bukkit/src/main/java/com/plotsquared/bukkit/util/BukkitUtil.java b/Bukkit/src/main/java/com/plotsquared/bukkit/util/BukkitUtil.java index d73a11a54..5acdde298 100644 --- a/Bukkit/src/main/java/com/plotsquared/bukkit/util/BukkitUtil.java +++ b/Bukkit/src/main/java/com/plotsquared/bukkit/util/BukkitUtil.java @@ -116,9 +116,11 @@ public class BukkitUtil extends WorldUtil { private static Player lastPlayer = null; private static BukkitPlayer lastPlotPlayer = null; - public static void removePlayer(String player) { + public static void removePlayer(UUID uuid) { lastPlayer = null; lastPlotPlayer = null; + // Make sure that it's removed internally + PlotSquared.imp().getPlayerManager().removePlayer(uuid); } public static PlotPlayer getPlayer(@NonNull final OfflinePlayer op) { diff --git a/Core/src/main/java/com/plotsquared/core/plot/Plot.java b/Core/src/main/java/com/plotsquared/core/plot/Plot.java index 83e616e9f..0d756ac6c 100644 --- a/Core/src/main/java/com/plotsquared/core/plot/Plot.java +++ b/Core/src/main/java/com/plotsquared/core/plot/Plot.java @@ -2917,7 +2917,7 @@ public class Plot { */ public void reEnter() { TaskManager.runTaskLater(() -> { - for (PlotPlayer pp : Plot.this.getPlayersInPlot()) { + for (PlotPlayer pp : Plot.this.getPlayersInPlot()) { PlotListener.plotExit(pp, Plot.this); PlotListener.plotEntry(pp, Plot.this); } @@ -2925,18 +2925,19 @@ public class Plot { } public void debug(@NotNull final String message) { - final Collection> players = PlotPlayer.getDebugModePlayersInPlot(this); - if (players.isEmpty()) { - return; - } - final String string = Captions.PLOT_DEBUG.getTranslated().replace("%plot%", this.toString()) - .replace("%message%", message); - for (final PlotPlayer player : players) { - if (isOwner(player.getUUID()) || - Permissions.hasPermission(player, Captions.PERMISSION_ADMIN_DEBUG_OTHER)) { - player.sendMessage(string); + try { + final Collection> players = PlotPlayer.getDebugModePlayersInPlot(this); + if (players.isEmpty()) { + return; } - } + final String string = + Captions.PLOT_DEBUG.getTranslated().replace("%plot%", this.toString()).replace("%message%", message); + for (final PlotPlayer player : players) { + if (isOwner(player.getUUID()) || Permissions.hasPermission(player, Captions.PERMISSION_ADMIN_DEBUG_OTHER)) { + player.sendMessage(string); + } + } + } catch (final Exception ignored) {} } /** diff --git a/Core/src/main/java/com/plotsquared/core/util/PlayerManager.java b/Core/src/main/java/com/plotsquared/core/util/PlayerManager.java index 4af113ca6..8c4ec1bea 100644 --- a/Core/src/main/java/com/plotsquared/core/util/PlayerManager.java +++ b/Core/src/main/java/com/plotsquared/core/util/PlayerManager.java @@ -30,6 +30,7 @@ import com.plotsquared.core.player.PlotPlayer; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -55,6 +56,17 @@ public abstract class PlayerManager

, T> { } } + /** + * Remove a player from the player map + * + * @param uuid Player to remove + */ + public void removePlayer(@NotNull final UUID uuid) { + synchronized (playerLock) { + this.playerMap.remove(uuid); + } + } + /** * Get the player from its UUID if it is stored in the player map. * @@ -110,7 +122,7 @@ public abstract class PlayerManager

, T> { } } - @NotNull protected abstract P createPlayer(@NotNull final UUID uuid); + @NotNull public abstract P createPlayer(@NotNull final UUID uuid); /** * Get an an offline player object from the player's UUID @@ -134,7 +146,7 @@ public abstract class PlayerManager

, T> { * @return Unmodifiable collection of players */ public Collection

getPlayers() { - return Collections.unmodifiableCollection(this.playerMap.values()); + return Collections.unmodifiableCollection(new ArrayList<>(this.playerMap.values())); }