Fixing like 10 memory leaks Fixes #4670

Changelog.txt

@@ -1,4 +1,10 @@
 Version 2.1.206
+    Fixed a memory leak involving Rupture
+    Fixed a memory leak involving Dodge
+    Fixed a memory leak involving Endermen and block pickups
+    Fixed a memory leak from plugin conflicts when double drops get activated
+    Fixed a memory leak that required a specific config.yml setup with mob health bars
+    You can no longer set mob health bars to -1 in config.yml to set it to display permanently (this was problematic behavior)
     Fixed a bug preventing Action Bar messages from showing
     Fixed a bug where Alchemy XP wasn't being granted
     Lowered the default volume of level up from .75 to .3 in sounds.yml (delete sounds.yml to get this change automagically)

src/main/java/com/gmail/nossr50/config/GeneralConfig.java

@@ -193,7 +193,7 @@ public class GeneralConfig extends AutoUpdateConfigLoader {
         }
     }
 
-    public int getMobHealthbarTime() { return config.getInt("Mob_Healthbar.Display_Time", 3); }
+    public int getMobHealthbarTime() { return Math.max(1, config.getInt("Mob_Healthbar.Display_Time", 3)); }
 
     /* Scoreboards */
     public boolean getScoreboardsEnabled() { return config.getBoolean("Scoreboard.UseScoreboards", true); }

src/main/java/com/gmail/nossr50/listeners/BlockListener.java

@@ -11,6 +11,7 @@ import com.gmail.nossr50.datatypes.skills.SuperAbilityType;
 import com.gmail.nossr50.datatypes.skills.ToolType;
 import com.gmail.nossr50.events.fake.FakeBlockBreakEvent;
 import com.gmail.nossr50.events.fake.FakeBlockDamageEvent;
+import com.gmail.nossr50.events.fake.FakeEvent;
 import com.gmail.nossr50.mcMMO;
 import com.gmail.nossr50.skills.alchemy.Alchemy;
 import com.gmail.nossr50.skills.excavation.ExcavationManager;
@@ -46,9 +47,16 @@ public class BlockListener implements Listener {
         this.plugin = plugin;
     }
 
-    @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true)
+    @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = false)
     public void onBlockDropItemEvent(BlockDropItemEvent event)
     {
+        //Make sure we clean up metadata on these blocks
+        if(event.isCancelled()) {
+            if(event.getBlock().hasMetadata(MetadataConstants.METADATA_KEY_BONUS_DROPS))
+                event.getBlock().removeMetadata(MetadataConstants.METADATA_KEY_BONUS_DROPS, plugin);
+            return;
+        }
+
         //Track how many "things" are being dropped
         HashSet<Material> uniqueMaterials = new HashSet<>();
         boolean dontRewardTE = false; //If we suspect TEs are mixed in with other things don't reward bonus drops for anything that isn't a block
@@ -318,21 +326,27 @@ public class BlockListener implements Listener {
     @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true)
     public void onBlockBreak(BlockBreakEvent event) {
         /* WORLD BLACKLIST CHECK */
-        if(WorldBlacklist.isWorldBlacklisted(event.getBlock().getWorld()))
+        Block block = event.getBlock();
+
+        if (event instanceof FakeBlockBreakEvent) {
             return;
+        }
+
+        if(WorldBlacklist.isWorldBlacklisted(block.getWorld())) {
+            BlockUtils.cleanupBlockMetadata(block);
+            return;
+        }
 
         /* WORLD GUARD MAIN FLAG CHECK */
         if(WorldGuardUtils.isWorldGuardLoaded())
         {
-            if(!WorldGuardManager.getInstance().hasMainFlag(event.getPlayer()))
+            if(!WorldGuardManager.getInstance().hasMainFlag(event.getPlayer())) {
+                BlockUtils.cleanupBlockMetadata(block);
                 return;
+            }
         }
 
-        if (event instanceof FakeBlockBreakEvent) {
-            return;
-        }
-
-        BlockState blockState = event.getBlock().getState();
+        BlockState blockState = block.getState();
         Location location = blockState.getLocation();
 
 //        if (!BlockUtils.shouldBeWatched(blockState)) {
@@ -347,6 +361,7 @@ public class BlockListener implements Listener {
         Player player = event.getPlayer();
 
         if (!UserManager.hasPlayerDataKey(player) || player.getGameMode() == GameMode.CREATIVE) {
+            BlockUtils.cleanupBlockMetadata(block);
             return;
         }
 
@@ -355,7 +370,8 @@ public class BlockListener implements Listener {
         //Check if profile is loaded
         if(mcMMOPlayer == null) {
             /* Remove metadata from placed watched blocks */
-            mcMMO.getPlaceStore().setFalse(blockState);
+
+            BlockUtils.cleanupBlockMetadata(block);
             return;
         }
 
@@ -417,7 +433,7 @@ public class BlockListener implements Listener {
         }
 
         /* Remove metadata from placed watched blocks */
-        mcMMO.getPlaceStore().setFalse(blockState);
+        BlockUtils.cleanupBlockMetadata(block);
     }
 
     /**
@@ -427,6 +443,9 @@ public class BlockListener implements Listener {
      */
     @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true)
     public void onBlockBreakHigher(BlockBreakEvent event) {
+        if(event instanceof FakeEvent)
+            return;
+
         /* WORLD BLACKLIST CHECK */
         if(WorldBlacklist.isWorldBlacklisted(event.getBlock().getWorld()))
             return;
@@ -438,10 +457,6 @@ public class BlockListener implements Listener {
                 return;
         }
 
-        if (event instanceof FakeBlockBreakEvent) {
-            return;
-        }
-
         Player player = event.getPlayer();
 
         if (!UserManager.hasPlayerDataKey(player) || player.getGameMode() == GameMode.CREATIVE) {

src/main/java/com/gmail/nossr50/listeners/EntityListener.java

@@ -11,6 +11,7 @@ import com.gmail.nossr50.events.fake.FakeEntityTameEvent;
 import com.gmail.nossr50.events.skills.rupture.McMMOEntityDamageByRuptureEvent;
 import com.gmail.nossr50.mcMMO;
 import com.gmail.nossr50.party.PartyManager;
+import com.gmail.nossr50.runnables.TravelingBlockMetaCleanup;
 import com.gmail.nossr50.skills.archery.Archery;
 import com.gmail.nossr50.skills.mining.BlastMining;
 import com.gmail.nossr50.skills.mining.MiningManager;
@@ -243,9 +244,12 @@ public class EntityListener implements Listener {
                 mcMMO.getPlaceStore().setFalse(block);
 
                 entity.setMetadata(MetadataConstants.METADATA_KEY_TRAVELING_BLOCK, MetadataConstants.MCMMO_METADATA_VALUE);
+                TravelingBlockMetaCleanup metaCleanupTask = new TravelingBlockMetaCleanup(entity, pluginRef);
+                metaCleanupTask.runTaskTimer(pluginRef, 20, 20*60); //6000 ticks is 5 minutes
             }
             else if (isTracked) {
                 mcMMO.getPlaceStore().setTrue(block);
+                entity.removeMetadata(MetadataConstants.METADATA_KEY_TRAVELING_BLOCK, pluginRef);
             }
         } else if ((block.getType() == Material.REDSTONE_ORE || block.getType().getKey().getKey().equalsIgnoreCase("deepslate_redstone_ore"))) {
             //Redstone ore fire this event and should be ignored

src/main/java/com/gmail/nossr50/runnables/MobDodgeMetaCleanup.java

@@ -0,0 +1,27 @@
+package com.gmail.nossr50.runnables;
+
+import com.gmail.nossr50.mcMMO;
+import com.gmail.nossr50.util.MetadataConstants;
+import org.bukkit.entity.Mob;
+import org.bukkit.scheduler.BukkitRunnable;
+import org.jetbrains.annotations.NotNull;
+
+public class MobDodgeMetaCleanup extends BukkitRunnable {
+    private final @NotNull Mob mob;
+    private final @NotNull mcMMO pluginRef;
+
+    public MobDodgeMetaCleanup(@NotNull Mob mob, @NotNull mcMMO pluginRef) {
+        this.mob = mob;
+        this.pluginRef = pluginRef;
+    }
+
+    @Override
+    public void run() {
+        if(!mob.isValid() || mob.getTarget() == null) {
+            mob.removeMetadata(MetadataConstants.METADATA_KEY_DODGE_TRACKER, pluginRef);
+            this.cancel();
+        } else if (!mob.hasMetadata(MetadataConstants.METADATA_KEY_DODGE_TRACKER)) {
+            this.cancel();
+        }
+    }
+}

src/main/java/com/gmail/nossr50/runnables/MobHealthDisplayUpdaterTask.java

@@ -14,9 +14,9 @@ public class MobHealthDisplayUpdaterTask extends BukkitRunnable {
 
     @Override
     public void run() {
-        if (target.hasMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME_KEY)) {
-            target.setCustomName(target.getMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME_KEY).get(0).asString());
-            target.removeMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME_KEY, mcMMO.p);
+        if (target.hasMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME)) {
+            target.setCustomName(target.getMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME).get(0).asString());
+            target.removeMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME, mcMMO.p);
         }
 
         if (target.hasMetadata(MetadataConstants.METADATA_KEY_NAME_VISIBILITY)) {

src/main/java/com/gmail/nossr50/runnables/TravelingBlockMetaCleanup.java

@@ -0,0 +1,27 @@
+package com.gmail.nossr50.runnables;
+
+import com.gmail.nossr50.mcMMO;
+import com.gmail.nossr50.util.MetadataConstants;
+import org.bukkit.entity.Entity;
+import org.bukkit.scheduler.BukkitRunnable;
+import org.jetbrains.annotations.NotNull;
+
+public class TravelingBlockMetaCleanup extends BukkitRunnable {
+    private final @NotNull Entity entity;
+    private final @NotNull mcMMO pluginRef;
+
+    public TravelingBlockMetaCleanup(@NotNull Entity entity, @NotNull mcMMO pluginRef) {
+        this.entity = entity;
+        this.pluginRef = pluginRef;
+    }
+
+    @Override
+    public void run() {
+        if(!entity.isValid()) {
+            entity.removeMetadata(MetadataConstants.METADATA_KEY_TRAVELING_BLOCK, pluginRef);
+            this.cancel();
+        } else if (!entity.hasMetadata(MetadataConstants.METADATA_KEY_TRAVELING_BLOCK)) {
+            this.cancel();
+        }
+    }
+}

src/main/java/com/gmail/nossr50/runnables/skills/RuptureTask.java

@@ -119,6 +119,7 @@ public class RuptureTask extends BukkitRunnable {
 //
 //        targetEntity.removeMetadata(mcMMO.RUPTURE_META_KEY, mcMMO.p);
 
+        targetEntity.removeMetadata(MetadataConstants.METADATA_KEY_RUPTURE, mcMMO.p);
         this.cancel(); //Task no longer needed
     }
 

src/main/java/com/gmail/nossr50/skills/acrobatics/AcrobaticsManager.java

@@ -8,6 +8,7 @@ import com.gmail.nossr50.datatypes.player.McMMOPlayer;
 import com.gmail.nossr50.datatypes.skills.PrimarySkillType;
 import com.gmail.nossr50.datatypes.skills.SubSkillType;
 import com.gmail.nossr50.mcMMO;
+import com.gmail.nossr50.runnables.MobDodgeMetaCleanup;
 import com.gmail.nossr50.skills.SkillManager;
 import com.gmail.nossr50.util.MetadataConstants;
 import com.gmail.nossr50.util.Misc;
@@ -21,6 +22,7 @@ import com.gmail.nossr50.util.skills.SkillUtils;
 import org.bukkit.Location;
 import org.bukkit.entity.Entity;
 import org.bukkit.entity.LightningStrike;
+import org.bukkit.entity.Mob;
 import org.bukkit.entity.Player;
 import org.bukkit.metadata.FixedMetadataValue;
 import org.bukkit.metadata.MetadataValue;
@@ -100,20 +102,22 @@ public class AcrobaticsManager extends SkillManager {
             }
 
             if (SkillUtils.cooldownExpired(mmoPlayer.getRespawnATS(), Misc.PLAYER_RESPAWN_COOLDOWN_SECONDS)) {
-                if(!(attacker instanceof Player)) {
+                if(attacker instanceof Mob) {
+                    Mob mob = (Mob) attacker;
                     //Check to see how many dodge XP rewards this mob has handed out
-                    if(attacker.hasMetadata(MetadataConstants.METADATA_KEY_DODGE_TRACKER) && ExperienceConfig.getInstance().isAcrobaticsExploitingPrevented()) {
+                    if(mob.hasMetadata(MetadataConstants.METADATA_KEY_DODGE_TRACKER) && ExperienceConfig.getInstance().isAcrobaticsExploitingPrevented()) {
                         //If Dodge XP has been handed out 5 times then consider it being exploited
-                        MetadataValue metadataValue = attacker.getMetadata(MetadataConstants.METADATA_KEY_DODGE_TRACKER).get(0);
-                        int count = attacker.getMetadata(MetadataConstants.METADATA_KEY_DODGE_TRACKER).get(0).asInt();
+                        MetadataValue metadataValue = mob.getMetadata(MetadataConstants.METADATA_KEY_DODGE_TRACKER).get(0);
+                        int count = metadataValue.asInt();
 
                         if(count <= 5) {
                             applyXpGain((float) (damage * Acrobatics.dodgeXpModifier), XPGainReason.PVE);
-                            attacker.setMetadata(MetadataConstants.METADATA_KEY_DODGE_TRACKER, new FixedMetadataValue(mcMMO.p, count + 1));
+                            mob.setMetadata(MetadataConstants.METADATA_KEY_DODGE_TRACKER, new FixedMetadataValue(mcMMO.p, count + 1));
+                            MobDodgeMetaCleanup metaCleanupTask = new MobDodgeMetaCleanup(mob, mcMMO.p);
+                            metaCleanupTask.runTaskTimer(mcMMO.p, 20, 20*60); //one minute
                         }
                     } else {
                         applyXpGain((float) (damage * Acrobatics.dodgeXpModifier), XPGainReason.PVE);
-                        attacker.setMetadata(MetadataConstants.METADATA_KEY_DODGE_TRACKER, new FixedMetadataValue(mcMMO.p, 1));
                     }
                 }
             }

src/main/java/com/gmail/nossr50/skills/herbalism/HerbalismManager.java

@@ -209,18 +209,21 @@ public class HerbalismManager extends SkillManager {
     public void processHerbalismBlockBreakEvent(BlockBreakEvent blockBreakEvent) {
         Player player = getPlayer();
 
+        Block block = blockBreakEvent.getBlock();
+
         if (mcMMO.p.getGeneralConfig().getHerbalismPreventAFK() && player.isInsideVehicle()) {
+            if(block.hasMetadata(MetadataConstants.METADATA_KEY_REPLANT)) {
+                block.removeMetadata(MetadataConstants.METADATA_KEY_REPLANT, mcMMO.p);
+            }
             return;
         }
 
         //Check if the plant was recently replanted
-        if(blockBreakEvent.getBlock().getBlockData() instanceof Ageable) {
-            Ageable ageableCrop = (Ageable) blockBreakEvent.getBlock().getBlockData();
-
-            if(blockBreakEvent.getBlock().getMetadata(MetadataConstants.METADATA_KEY_REPLANT).size() >= 1) {
-                if(blockBreakEvent.getBlock().getMetadata(MetadataConstants.METADATA_KEY_REPLANT).get(0).asBoolean()) {
+        if(block.getBlockData() instanceof Ageable ageableCrop) {
+            if(block.getMetadata(MetadataConstants.METADATA_KEY_REPLANT).size() >= 1) {
+                if(block.getMetadata(MetadataConstants.METADATA_KEY_REPLANT).get(0).asBoolean()) {
                     if(isAgeableMature(ageableCrop)) {
-                        blockBreakEvent.getBlock().removeMetadata(MetadataConstants.METADATA_KEY_REPLANT, mcMMO.p);
+                        block.removeMetadata(MetadataConstants.METADATA_KEY_REPLANT, mcMMO.p);
                     } else {
                         //Crop is recently replanted to back out of destroying it
                         blockBreakEvent.setCancelled(true);
@@ -314,7 +317,7 @@ public class HerbalismManager extends SkillManager {
             DelayedHerbalismXPCheckTask delayedHerbalismXPCheckTask = new DelayedHerbalismXPCheckTask(mmoPlayer, delayedChorusBlocks);
 
             //Large delay because the tree takes a while to break
-            delayedHerbalismXPCheckTask.runTaskLater(mcMMO.p, 20); //Calculate Chorus XP + Bonus Drops 1 tick later
+            delayedHerbalismXPCheckTask.runTaskLater(mcMMO.p, 0); //Calculate Chorus XP + Bonus Drops 1 tick later
         }
     }
 

src/main/java/com/gmail/nossr50/util/BlockUtils.java

@@ -39,6 +39,19 @@ public final class BlockUtils {
             blockState.setMetadata(MetadataConstants.METADATA_KEY_BONUS_DROPS, new BonusDropMeta(1, mcMMO.p));
     }
 
+    /**
+     * Cleans up some block metadata when a block breaks and the metadata is no longer needed
+     * This also sets the blocks coords to false in our chunk store
+     * @param block target block
+     */
+    public static void cleanupBlockMetadata(Block block) {
+        if(block.hasMetadata(MetadataConstants.METADATA_KEY_REPLANT)) {
+            block.removeMetadata(MetadataConstants.METADATA_KEY_REPLANT, mcMMO.p);
+        }
+
+        mcMMO.getPlaceStore().setFalse(block);
+    }
+
     /**
      * Marks a block to drop extra copies of items
      * @param blockState target blockstate

src/main/java/com/gmail/nossr50/util/MetadataConstants.java

@@ -42,7 +42,7 @@ public class MetadataConstants {
     public static final @NotNull String METADATA_KEY_PLAYER_TAMED_MOB = "mcmmo_player_tamed_mob";
     public static final @NotNull String METADATA_KEY_VILLAGER_TRADE_ORIGIN_ITEM = "mcmmo_villager_trade_origin_item";
     public static final @NotNull String METADATA_KEY_EXPLOITED_ENDERMEN = "mcmmo_exploited_endermen";
-    public static final @NotNull String METADATA_KEY_CUSTOM_NAME_KEY = "mcmmo_custom_name";
+    public static final @NotNull String METADATA_KEY_CUSTOM_NAME = "mcmmo_custom_name";
     public static final @NotNull String METADATA_KEY_OLD_NAME_KEY = "mcmmo_old_name";
     public static final @NotNull String METADATA_KEY_RUPTURE = "mcmmo_rupture";
 
@@ -61,10 +61,11 @@ public class MetadataConstants {
         temp.add(MetadataConstants.METADATA_KEY_PLAYER_BRED_MOB);
         temp.add(MetadataConstants.METADATA_KEY_PLAYER_TAMED_MOB);
         temp.add(MetadataConstants.METADATA_KEY_EXPLOITED_ENDERMEN);
-        temp.add(MetadataConstants.METADATA_KEY_CUSTOM_NAME_KEY);
+        temp.add(MetadataConstants.METADATA_KEY_CUSTOM_NAME);
         temp.add(MetadataConstants.METADATA_KEY_RUPTURE);
         temp.add(MetadataConstants.METADATA_KEY_EXPLOSION_FROM_RUPTURE);
         temp.add(MetadataConstants.METADATA_KEY_OLD_NAME_KEY);
+        temp.add(MetadataConstants.METADATA_KEY_DODGE_TRACKER);
 
         MOB_METADATA_KEYS = ImmutableSet.copyOf(temp);
     }

src/main/java/com/gmail/nossr50/util/MobHealthbarUtils.java

@@ -73,11 +73,11 @@ public final class MobHealthbarUtils {
             boolean updateName = !ChatColor.stripColor(oldName).equalsIgnoreCase(ChatColor.stripColor(newName));
 
             if (updateName) {
-                target.setMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME_KEY, new FixedMetadataValue(mcMMO.p, oldName));
+                target.setMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME, new FixedMetadataValue(mcMMO.p, oldName));
                 target.setMetadata(MetadataConstants.METADATA_KEY_NAME_VISIBILITY, new FixedMetadataValue(mcMMO.p, oldNameVisible));
             }
-            else if (!target.hasMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME_KEY)) {
-                target.setMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME_KEY, new FixedMetadataValue(mcMMO.p, ""));
+            else if (!target.hasMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME)) {
+                target.setMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME, new FixedMetadataValue(mcMMO.p, ""));
                 target.setMetadata(MetadataConstants.METADATA_KEY_NAME_VISIBILITY, new FixedMetadataValue(mcMMO.p, false));
             }
 

src/main/java/com/gmail/nossr50/util/TransientMetadataTools.java

@@ -13,9 +13,9 @@ public class TransientMetadataTools {
 
     public void cleanLivingEntityMetadata(@NotNull LivingEntity entity) {
         //Since it's not written anywhere, apparently the GC won't touch objects with metadata still present on them
-        if (entity.hasMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME_KEY)) {
-            entity.setCustomName(entity.getMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME_KEY).get(0).asString());
-            entity.removeMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME_KEY, pluginRef);
+        if (entity.hasMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME)) {
+            entity.setCustomName(entity.getMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME).get(0).asString());
+            entity.removeMetadata(MetadataConstants.METADATA_KEY_CUSTOM_NAME, pluginRef);
         }
 
 //        if(entity.hasMetadata(MetadataConstants.METADATA_KEY_OLD_NAME_KEY)) {

src/main/resources/config.yml

@@ -149,7 +149,6 @@ Scoreboard:
 Mob_Healthbar:
     # Enabled: Whether or not the feature is enabled at all
     # Display_Type: Per player Default display for mob health bars - HEARTS, BAR, or DISABLED
-    # Display_Time: Amount of time (in seconds) to display. To display permanently, set to -1
     Enabled: true
     Display_Type: HEARTS
     Display_Time: 3