[UNSOLVED] ConcurrentModificationException, no idea what the problem is

Discussion in 'Plugin Development' started by ItsHarry, Feb 17, 2012.

Thread Status:
Not open for further replies.
  1. Offline

    ItsHarry

    I'm getting a ConcurrentModificationException on the line "HERE HERE"

    Code:
    final List<String> userRedeems = redeems.get(username);
                if (kits == null) {
                    sender.sendMessage(ChatColor.RED + "Kits file is empty, please tell an admin.");
                    return true;
                }
                if (userRedeems.isEmpty()) {
                    sender.sendMessage(ChatColor.RED + "You don't have any pending redeems.");
                    return true;
                }
                final List<String> newUserRedeems = userRedeems;
                for (final String s : userRedeems) { //HERE HERE HERE HERE HERE HERE
                    final String t = s;
                    if (!kits.containsKey(t)) {
                        sender.sendMessage(ChatColor.RED + "Kit " + t + " was not found. Please tell an admin.");
                        continue;
                    }
                    final ItemStack[] pi = kits.get(t).getContents();
                    for (final ItemStack is : pi) {
                        if (is == null) {
                            continue;
                        }
                        player.getWorld().dropItemNaturally(player.getLocation(), is);
                    }
                    newUserRedeems.remove(t);
                }
                redeems.put(username, newUserRedeems);
                sender.sendMessage(ChatColor.GREEN + "There you go!");
    Anyone know why?

    I even put String t = s; to test, but it gives the same error.

    Btw. All items are dropped, but "There you go" is not sent and it gives me "An internal error occured" when using this command.
     
  2. It's happening because you try to modify the list while it's in use (in the for loop)

    try using the list's iterator instead.
     
  3. Offline

    ItsHarry

    But I'm not modifying the list anywhere in the for loop
     
  4. Yes, you are, right there:
    Code:
                for (final String s : userRedeems) {
                    // [SNIP]
                    newUserRedeems.remove(t); // here, you modify the list you are looping through, not possible!
                }
    Solution: use userRedeems.iterator() and then call iterator.remove() to delete the current entry from the list (google "java iterator" if you need more information).
     
  5. Offline

    ItsHarry

    That's not true! The for loop is using "userRedeems" while I remove from the variable "newUserRedeems". Not the same list :(
     
  6. Offline

    Father Of Time

    They are one in the same, "newUserRedeems" is a reference to the memory allocation for "userRedeems". You are populating newUserRedeems with a reference to userRedeems, not copying it with a clone of the list.

    If you want a copy you need to do something like:

    Code:
    ArrayList newArrayList =(ArrayList) oldArrayList.clone();
    
    That way instead of both variables being populated with the same information in memory you will allocate two different sections of memory to the two different variables and "clone" the content from userRedeems to newUserRedeems...

    You might want to do a little reading into references vs copies, good luck with your project!
     
    ItsHarry likes this.
  7. Offline

    Zimp

    This means that newUserRedeems is pointing to the same reference as userRedeems. What you really need to do here is:

    Code:
    final List<String> newUserRedeems = new ArrayList<String>(userRedeems);
     
  8. Offline

    ItsHarry

    Ah!!! Thank you, I never knew that, I always though when you made a new variable, it would allocate new memory :p
     
  9. Offline

    Father Of Time

    Yea, I didn't know this myself when I first came to Java and had to have it explained to me, but once you understand the concept its a very easy thing to prepare for and work around, and it actually is good that it works this way so that you can get handlers and assign them to a local variable and work with a single instance of a handler as appose to making new instances of it every time you get it (to name one benefit).

    Good luck with your project!
     
    ItsHarry likes this.
Thread Status:
Not open for further replies.

Share This Page