Solved ConcurrentModificationException

Hello. I am writting a simple plugin that fires kittens from a bow then explodes them into a firework (Adorable, right?). I am getting this exception:

Stack Trace (open) Stack Trace (close)

Code:

20:40:37 [SEVERE] java.util.ConcurrentModificationException
20:40:37 [SEVERE]      at java.util.HashMap$HashIterator.nextEntry(Unknown Sour
ce)
20:40:37 [SEVERE]      at java.util.HashMap$KeyIterator.next(Unknown Source)
20:40:37 [SEVERE]      at net.minecraft.server.v1_6_R2.EntityTracker.updatePlay
ers(EntityTracker.java:149)
20:40:37 [SEVERE]      at net.minecraft.server.v1_6_R2.MinecraftServer.t(Minecr
aftServer.java:581)
20:40:37 [SEVERE]      at net.minecraft.server.v1_6_R2.DedicatedServer.t(Dedica
tedServer.java:226)
20:40:37 [SEVERE]      at net.minecraft.server.v1_6_R2.MinecraftServer.s(Minecr
aftServer.java:486)
20:40:37 [SEVERE]      at net.minecraft.server.v1_6_R2.MinecraftServer.run(Mine
craftServer.java:419)
20:40:37 [SEVERE]      at net.minecraft.server.v1_6_R2.ThreadServerApplication.
run(SourceFile:582)
20:40:37 [SEVERE] Encountered an unexpected exception ConcurrentModificationExce
ption
java.util.ConcurrentModificationException
        at java.util.HashMap$HashIterator.nextEntry(Unknown Source)
        at java.util.HashMap$KeyIterator.next(Unknown Source)
        at net.minecraft.server.v1_6_R2.EntityTracker.updatePlayers(EntityTracke
r.java:149)
        at net.minecraft.server.v1_6_R2.MinecraftServer.t(MinecraftServer.java:5
81)
        at net.minecraft.server.v1_6_R2.DedicatedServer.t(DedicatedServer.java:2
26)
        at net.minecraft.server.v1_6_R2.MinecraftServer.s(MinecraftServer.java:4
86)
        at net.minecraft.server.v1_6_R2.MinecraftServer.run(MinecraftServer.java
:419)
        at net.minecraft.server.v1_6_R2.ThreadServerApplication.run(SourceFile:5
82)

but it does not tell me where I have gone wrong. This is my code:

Main (open) Main (close)

Code:java

import java.util.ArrayList;
import java.util.logging.Level;
import java.util.logging.Logger;
 
import org.bukkit.ChatColor;
import org.bukkit.Material;
import org.bukkit.command.Command;
import org.bukkit.command.CommandSender;
import org.bukkit.craftbukkit.v1_6_R2.entity.CraftEntity;
import org.bukkit.entity.Player;
import org.bukkit.inventory.ItemStack;
import org.bukkit.inventory.meta.ItemMeta;
import org.bukkit.plugin.java.JavaPlugin;
 
public class Main extends JavaPlugin {
 
    static Logger logger = Logger.getLogger("Minecraft");
    static ArrayList<CraftEntity> cats = new ArrayList<CraftEntity>();
 
    @Override
    public void onEnable() {
        getServer().getPluginManager().registerEvents(new BowListener(), this);
        getServer().getPluginManager().registerEvents(new PlayerListener(), this);
    }
 
    @Override
    public void onDisable() {
        int catCount = 0;
        for (CraftEntity c : cats) {
            c.remove();
            catCount++;
        }
        logger.log(Level.INFO, "[Kitty Bow] Removed " + catCount + " fluffy kitties!");
    }
 
    @Override
    public boolean onCommand(CommandSender sender, Command cmd, String CommandLabel, String[] args) {
        Player player = (Player) sender;
 
        ItemStack kittyBow = new ItemStack(Material.BOW);
 
        ArrayList<String> kbLore = new ArrayList<String>();
        kbLore.add(rainbow("Kitty Bow"));
        ItemMeta kbMeta = kittyBow.getItemMeta();
 
        kbMeta.setLore(kbLore);
 
        kittyBow.setItemMeta(kbMeta);
 
        player.getInventory().addItem(kittyBow);
        return false;
    }
 
    private String rainbow(String string) {
        String rainbowised = "";
        ChatColor[] colours = {ChatColor.RED, ChatColor.GOLD, ChatColor.YELLOW, ChatColor.GREEN, ChatColor.BLUE, ChatColor.DARK_BLUE, ChatColor.LIGHT_PURPLE};
        for (int i = 0; i < string.length(); i++) {
            rainbowised = rainbowised + colours[listNum(i, colours.length)] + string.charAt(i);
        }
        return rainbowised;
    }
 
    private int listNum(int get, int size) {
        int fin = get;
        while (fin >= size) {
            fin -= size;
        }
        return fin;
    }
 
}
 

BowListener (open) BowListener (close)

Code:java

import net.minecraft.server.v1_6_R2.EntityOcelot;
import org.bukkit.ChatColor;
import org.bukkit.Color;
import org.bukkit.FireworkEffect;
import org.bukkit.FireworkEffect.Type;
import org.bukkit.Location;
import org.bukkit.craftbukkit.v1_6_R2.entity.CraftEntity;
import org.bukkit.entity.Arrow;
import org.bukkit.entity.EntityType;
import org.bukkit.entity.LivingEntity;
import org.bukkit.entity.Player;
import org.bukkit.event.EventHandler;
import org.bukkit.event.Listener;
import org.bukkit.event.entity.ProjectileLaunchEvent;
import org.bukkit.potion.PotionEffect;
import org.bukkit.potion.PotionEffectType;
 
public class BowListener implements Listener {
 
    int time = 5;
 
    @EventHandler
    public void onLaunch(ProjectileLaunchEvent event) {
        if (event.getEntity() instanceof Arrow) {
            Arrow a = (Arrow) event.getEntity();
            if (a.getShooter() instanceof Player) {
                final Player p = (Player) a.getShooter();
                try {
                    if (ChatColor.stripColor(p.getItemInHand().getItemMeta().getLore().get(0)).equals("Kitty Bow")) {
                        event.setCancelled(true);
 
                        final CraftEntity craftCat = (CraftEntity) p.getWorld().spawnEntity(p.getLocation().add(0, 2, 0), EntityType.OCELOT);
                        final EntityOcelot cat = (EntityOcelot) craftCat.getHandle();
                        cat.setLocation(cat.locX, cat.locY, cat.locZ, 45, 45);
                        cat.setOwnerName(p.getName());
                        cat.setTamed(true);
                        cat.setSitting(true);
                        cat.setCatType(random(0, 2));
                        craftCat.setVelocity(a.getVelocity());
 
                        ((LivingEntity)craftCat).addPotionEffect(new PotionEffect(PotionEffectType.SLOW, time * 20, 30));
 
                        Main.cats.add(craftCat);
 
                        Thread t = new Thread(new Runnable() {
                            @Override
                            public void run() {
                                try {
                                    Thread.sleep(time * 1000);
                                } catch (InterruptedException IE) {
                                }
                                int ranType = random(0, 1);
                                Type type = null;
 
                                if (ranType == 0) {
                                    type = Type.BALL;
                                } else if (ranType == 1) {
                                    type = Type.BURST;
                                }
 
                                FireworkEffect effect = FireworkEffect.builder().flicker(false).withColor(Color.fromRGB(random(0, 255), random(0, 255), random(0, 255))).withFade(Color.fromRGB(random(0, 255), random(0, 255), random(0, 255))).with(type).trail(false).build();
 
                                try {
                                    InstantFirework.playFirework(p.getWorld(), new Location(p.getWorld(), cat.locX, cat.locY, cat.locZ), effect);
                                } catch (Exception e) {
                                    e.printStackTrace();
                                }
 
                                Main.cats.remove(craftCat);
                                craftCat.remove();
                            }
                        });
                        t.start();
                    }
                } catch (NullPointerException npe) {
                }
            }
        }
    }
 
    private int random(int Min, int Max) {
        return Min + (int)(Math.random() * ((Max - Min) + 1));
    }
 
}

PlayerListener (open) PlayerListener (close)

Code:java

import java.util.logging.Level;
 
import org.bukkit.craftbukkit.v1_6_R2.entity.CraftEntity;
import org.bukkit.entity.Tameable;
import org.bukkit.event.EventHandler;
import org.bukkit.event.Listener;
import org.bukkit.event.player.PlayerQuitEvent;
 
public class PlayerListener implements Listener {
 
    @EventHandler
    public void onPlayerLeave(PlayerQuitEvent e) {
        int catCount = 0;
        for (CraftEntity c : Main.cats) {
            if (((Tameable) c).getOwner().equals(e.getPlayer())) {
                c.remove();
                catCount++;
            }
        }
        if (catCount != 0) {
            Main.logger.log(Level.INFO, "Removed " + (catCount == 1 ? e.getPlayer().getName() + "'s cat" : catCount + " of " + e.getPlayer().getName() + "'s cats"));
        }
    }
 
}
 

InstantFirework (open) InstantFirework (close)

Code:java

import java.lang.reflect.Method;
 
import org.bukkit.FireworkEffect;
import org.bukkit.entity.Firework;
import org.bukkit.inventory.meta.FireworkMeta;
import org.bukkit.Location;
import org.bukkit.World;
 
public class InstantFirework {
 
    private static Method world_getHandle = null;
    private static Method nms_world_broadcastEntityEffect = null;
    private static Method firework_getHandle = null;
 
    public static void playFirework(World world, Location loc, FireworkEffect fe) throws Exception {
        Firework fw = (Firework) world.spawn(loc, Firework.class);
        Object nms_world = null;
        Object nms_firework = null;
 
        if(world_getHandle == null) {
            world_getHandle = getMethod(world.getClass(), "getHandle");
            firework_getHandle = getMethod(fw.getClass(), "getHandle");
        }
        nms_world = world_getHandle.invoke(world, (Object[]) null);
        nms_firework = firework_getHandle.invoke(fw, (Object[]) null);
        if(nms_world_broadcastEntityEffect == null) {
            nms_world_broadcastEntityEffect = getMethod(nms_world.getClass(), "broadcastEntityEffect");
        }
        FireworkMeta data = (FireworkMeta) fw.getFireworkMeta();
        data.clearEffects();
        data.setPower(1);
        data.addEffect(fe);
        fw.setFireworkMeta(data);
 
        nms_world_broadcastEntityEffect.invoke(nms_world, new Object[] {nms_firework, (byte) 17});
        fw.remove();
    }
 
    private static Method getMethod(Class<?> cl, String method) {
        for(Method m : cl.getMethods()) {
            if(m.getName().equals(method)) {
                return m;
            }
        }
        return null;
    }
 
}
 

Any help will be appriciated!
Thanks,
-Curly

 

That's because you're calling Bukkit API/NMS in asynchronous threads (line 45 in BowListener), just to delay execution of code. This is never a good idea, it can potentially cause the server to crash entirely, or even worse - overwrite the world files with corrupt data. Getting a ConcurrentModificationException is arguably the best possible outcome here, since at least you know you did something wrong before servers are randomly deadlocked.

So - schedule synchronous tasks instead. That way, the code will be executed after a delay like you want, but on the main server thread where you can call the Bukkit API to your hearts content.

 

Making BukkitAPI calls in an async thread is a big no no.
Use a Bukit Scheduler and like Comphenix said, use a synchronous task for this.
A delayed task like "Bukkit.getServer().getScheduler().runTaskLater()" should work for this.

But your exception is caused by this and similar code parts:

Code:

 for (CraftEntity c : Main.cats) {
if (((Tameable) c).getOwner().equals(e.getPlayer())) {
c.remove();
catCount++;
}
}

You are deleting entries from an ArrayList while looping through it. (in more than one occurrence)
You should use an Interator for this.

 

Well, the remove() method here simply marks the entity as dead, which will be handled during the next tick. It doesn't remove anything from any array list, though that is exactly what OP should do in onDisable(), to avoid any potential memory leaks.

If it was one of those mistakes, the exception would have occurred in the plugin itself, not in some arbitrary part of Minecraft. For that to occur, the plugin must have executed code concurrently with the main thread, which means abuse of threading.

 

curlyfries1999
A lotta things could go wrong in that new thread you started...
1. Print stack trace instead of doing nothing in the try catch
2. Why are you catching an NPE??
3. Bukkit methods should only be called from Sync tasks as overly emphasized
4. No need for a different thread

 

So, I need to replace the thread with a scheduler?

 

Code:java

 for (CraftEntity c : Main.cats.clone()) {
if (((Tameable) c).getOwner().equals(e.getPlayer())) {
Main.cats.remove(c);
//c.remove();
catCount++;
}
}

Use Main.cats.clone() to iterate over the the cloned Hash-map, because you can't delete an entity in hash map while iterating through it.

 

Look into http://jd.bukkit.org/dev/apidocs/org/bukkit/scheduler/BukkitRunnable.html

About the only differences would be you'd do a new BukkitRunnable instead of a new Thread(new Runnable, you'd get rid of the sleep at the start of the runnable, and instead of t.start() you'd do t.runTaskLater(plugin,ticks) where you get 20 ticks to a second (theoretically)

 

frymaster Thank you very much! That worked great!