[SOLVED] iterator.next() throws ConcurrentModificationException

Discussion in 'Plugin Development' started by V10lator, Dec 30, 2011.

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

    V10lator

    The code:
    Code:java
    1. // Strange bugfix: Even if we don't modify we get a
    2. // concurrent modification exception...
    3. // So let's use a iterator.
    4. Iterator<Entry<String, LinkedHashMap<String, Integer>>> eiter = plugin.queue.entrySet().iterator();
    5. while(eiter.hasNext())
    6. {
    7. Entry<String, LinkedHashMap<String, Integer>> e = eiter.next(); //Line 58

    Strange bugfix because the iterator should fix it (therefor I had the exception at the loop, which was a for each loop) ... Now the last line you see here is throwing the error:
    Code:
    2011-12-30 11:34:47 [WARNING] Task of 'V10lift' generated an exception
    java.util.ConcurrentModificationException
    	at java.util.HashMap$HashIterator.nextEntry(HashMap.java:810)
    	at java.util.HashMap$EntryIterator.next(HashMap.java:851)
    	at java.util.HashMap$EntryIterator.next(HashMap.java:849)
    	at de.V10lator.V10lift.MoveLift.run(MoveLift.java:58)
    	at org.bukkit.craftbukkit.scheduler.CraftScheduler.mainThreadHeartbeat(CraftScheduler.java:137)
    	at net.minecraft.server.MinecraftServer.w(MinecraftServer.java:493)
    	at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:425)
    	at net.minecraft.server.ThreadServerApplication.run(SourceFile:457)
    Before you ask: No, there's no other loop before and no, I'm not in an async task.
     
  2. Offline

    tkausl

    Some thread change the entrySet. May you copy the Entryset temporary.
     
  3. Offline

    V10lator

    The plugin doesn't have any async tasks and there are no other plugins hooking into it...

    //EDIT: Also, look at the codes: I create the entrySet() there, so how can another thread change it?
     
  4. Offline

    bergerkiller

    @V10lator you use the iterator of something outside that method:
    Code:
    Iterator<Entry<String, LinkedHashMap<String, Integer>>> eiter = plugin.queue.entrySet().iterator();
    plugin.queue
    Does your 'queue' map get accessed outside that method? From another thread? Inside the loop?
     
  5. Offline

    V10lator

    Let me show you some more code:
    in onEnable():
    Code:java
    1. server.getScheduler().scheduleSyncRepeatingTask(this, new MoveLift(this), 16L, 16L);

    and a bit more from the class I showed above:
    Code:java
    1. public void run()
    2. {
    3. Server s = plugin.getServer();
    4. // Strange bugfix: Even if we don't modify we get a
    5. // concurrent modification exception...
    6. // So let's use a iterator.
    7. Iterator<Entry<String, LinkedHashMap<String, Integer>>> eiter = plugin.queue.entrySet().iterator();
    8. while(eiter.hasNext())
    9. {
    10. Entry<String, LinkedHashMap<String, Integer>> e = eiter.next();

    If I search in that class for "plugin.queue" there's only one line:
    Code:java
    1. Iterator<Entry<String, LinkedHashMap<String, Integer>>> eiter = plugin.queue.entrySet().iterator();


    This is going to drive me crazy :'(
     
  6. Offline

    tkausl

    Post the whole while please.
     
  7. Offline

    V10lator

    @tkausl: The whole while loop has around 300 lines of not-jet-released code, so no. Just trust me that there is no other access to plugin.queue. If here's a member I trust which wants to revive the code I'll send it via PM. But be warned that it's spaghetti code (tuned for performance, not for readability).
     
  8. Offline

    Digi

    You can achieve readability with performance as the compiler doesn't care about spacing, that's not the case for trying to achieve small code size, that sacrifices readability first of all and could affect performance as well :}

    Anyway... if everything fails, start trial and error :} comment out parts of that loop and see if the problem persists... first you should comment the entire loop contents to be sure that the problem is in the loop and not somewhere else.
     
  9. Offline

    bergerkiller

    @V10lator 'in that class', it seems that the hashmap is located in your plugins' main class (you are accessing the static JavaPlugin plugin variable)
    Are you really sure that the queue variable is not used anywhere else in your entire project? Even if it's an event that uses it, it is possible that a plugin is accessing it from another thread. (therefore NoLagg's async access errors)

    Also, are you using Java 7? I have had something similar in TrainCarts where it threw an error at the remove or next iterator line...it proved to be an async access issue then.
     
  10. Offline

    mindless728

    are you adding and/or removing from the Collection while in the loop NOT using the iterator to do so, as that will also cause a ConcurrentModificationException to be called

    ie (not real java, just pseudo java, you will get the idea)
    Code:java
    1.  
    2. Iterator i = list.getIterator();
    3. while(i.hasNext()) {
    4. Object o = i.next();
    5. //do stuff
    6. list.remove(o);
    7. }
    8.  
     
  11. Offline

    V10lator

    You misinterpreted me. The code isn't designed to be small in codesize, it is designed to be fast in it's execution. So for example it doesn't use getters and setters but manipulates the objects directly.
    That's out of question. The error seems to be extremely rare and it's not clear what exactly triggers it. It came up one time in about 4 days (task executing it's stuff 24/7).

    //EDIT:
    No, I'm not. As I told: It's driving me crazy...

    Yes, it is used in events. /facepalm - Now I start to really understand your thread (you know which one I mean).
    So I can say this is not caused by my plugin?
    Not sure as this error wasn't thrown on my server, it was thrown on another server (while I was online) but the admin of it is sleeping right now.

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
     
    Last edited by a moderator: May 22, 2016
  12. Offline

    bergerkiller

    @V10lator yup you can sure say that. Even if it is called async ONCE, it is enough to ruin everything as this demonstrates :)
     
    V10lator likes this.
  13. Offline

    Digi

    You mean like:
    Code:
    CustomVector vec = new CustomVector();
    
    int x = vec.x; // instead of getX()
    ?
    Well, I use that and I find it verry readable... but, that is, if the variable names are right :}

    But no, I was saying that if you're making code for performance, you couldn't be stacking it tight, therefore it's spread out and readable as well :p since making code tight and not using spaces doesn't help performance, which I belive you know.

    Still, you should post the code either way because a fresh pair of eyes might see something wrong... and it doesn't really matter if it's "not-jet-released" ( :p ) code, because people will know that it's your code since you posted it first... unless you don't plan on releasing the source along with the final plugin.
     
  14. Offline

    bergerkiller

    @Digi I guess I could learn something from that...I do know that comments are not read and neither are whitespaces..yet I manage to make the tightest code structures :)
    Example
     
  15. Offline

    Digi

    @bergerkiller yes well, personal preferences are not negotiable :p The result is what usually counts tough.
     
Thread Status:
Not open for further replies.

Share This Page