[Solved] ConcurrentModificationException with iter.next()

Discussion in 'Plugin Development' started by V10lator, Sep 10, 2012.

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

    V10lator

  2. Offline

    desht

  3. Offline

    V10lator

    desht It is not async. There's no async mechanism left in BananaRegion. Also I'm not removing/adding entries to the list while iterating (except with iter.remove() - but that's allowed, your linked thread proves it).
     
  4. Offline

    desht

    Right, I checked your code - I can see you're not doing anything async in BananaRegion. The only way that iter.next() can be throwing a CME is if something has changed the underlying data structure after you created the iterator on it. Therefore, it must be some other plugin doing this, from a separate thread. I'd start by finding out what other plugins your user has installed...
     
  5. Offline

    V10lator

    desht I just asked that question. :)
     
  6. Offline

    nisovin

    It doesn't necessarily have to be caused by other plugin or another thread. It could be caused by any code you're calling that might modify your collections. For example, you call world.getBlockAt. If the chunk for that block is not loaded, it will be loaded, at which time your ChunkLoadEvent will fire, which I suspect modifies your collection object.
     
    desht likes this.
  7. Offline

    V10lator

    nisovin The only function which could cause this is your example: getBlockAt. - But the chunk there should be loaded as I'm listening for the ChunkUnloadEvent:
    Code:java
    1. @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true)
    2. public void onChunkUnload(ChunkUnloadEvent event)
    3. {
    4. Chunk chunk = event.getChunk();
    5. Sign s;
    6. for(BlockState e : chunk.getTileEntities())
    7. {
    8. if(e instanceof Sign)
    9. {
    10. s = (Sign)e;
    11. if(s.getLine(0).equalsIgnoreCase(plugin.signText))
    12. plugin.rr.unloadRegion(s);
    13. }
    14. }
    15. }

    Of course I also listen for block break events. But what if a plugin removes the sign (no event) and the chunk gets unloaded? But then it shouldn't pass the new ChunkLoadEvent (no BR sign -> no modification of the collection object).
    Anyway, I added this:
    Code:java
    1. while(iter2.hasNext())
    2. {
    3. region = iter2.next(); //TODO: Check for ConcurrentModificationException
    4. split = region.signHash.split(",");
    5. x = Integer.parseInt(split[0]);
    6. y = Integer.parseInt(split[1]);
    7. z = Integer.parseInt(split[2]);
    8. if(!world.isChunkLoaded(x>>4, z>>4))
    9. {
    10. if(plugin.debug)
    11. plugin.log.info("Chunk at "+(x>>4)+"/"+(z>>4)+" in world "+wn+" seems to have unloaded without notice. Fixing RAM structure.");
    12. iter2.remove();
    13. continue;
    14. }
    15. bs = world.getBlockAt(x, y, z).getState();

    Also I tried to protect the API:
    Code:java
    1. private void asyncCheck()
    2. {
    3. if(!Thread.currentThread().getName().equals("Server thread"))
    4. throw new UnsupportedOperationException("Some plugin tried to access BananaRegions API from another thread. THIS IS NOT A BUG OF BANANAREGION!");
    5. }

    Which I call at the beginning of every API function, that gives stacktraces like this:
    Code:
    11:19:11 [SCHWERWIEGEND] Exception in thread "pool-1-thread-2" 
    11:19:11 [SCHWERWIEGEND] org.apache.commons.lang.UnhandledException: Plugin BananaCheck v0.2 generated an exception while executing task 1878
        at org.bukkit.craftbukkit.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:56)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603)
        at java.lang.Thread.run(Thread.java:722)
    Caused by: java.lang.UnsupportedOperationException: Some plugin tried to access BananaRegions API from another thread. THIS IS NOT A BUG OF BANANAREGION!
        at de.V10lator.BananaRegion.BananaRegion_API.asyncCheck(BananaRegion_API.java:322)
        at de.V10lator.BananaRegion.BananaRegion_API.getOwners(BananaRegion_API.java:136)
        at de.V10lator.BananaCheck.BananaCheck.run(BananaCheck.java:29)
        at org.bukkit.craftbukkit.scheduler.CraftTask.run(CraftTask.java:52)
        at org.bukkit.craftbukkit.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:53)
        ... 3 more
    But I don't know how accurate my asyncCheck really is.

    has anyone any more ideas to slow the code down, eh, I mean to fix this? ^^
     
  8. Offline

    V10lator

    Okay, I had to add the async check to all listeners, too, then the plugin was identified. Something in that method causes bukkit to call a ChunkLoadEvent (and that method is called from an async task). Told the bug reporter to report it to the other plugin.
     
    desht likes this.
  9. Offline

    Bone008

    Oh my god. I don't know anything about neither the plugin nor the author nor the code, but something like this is just ignorant:
    Code:
                try {
                    store.Save();
                } catch(ConcurrentModificationException ex) {
                    store.lastID = Bukkit.getScheduler().scheduleAsyncDelayedTask(SignShop.getInstance(), this);
                }
    (at the bottom in DeferredSave)
    "Oh, we corrupted something by not caring about synchronization? Kay, no problem, let's just try it again, maybe it will happen to work next time!!!!!11111"
    Seriously?!
     
    V10lator likes this.
Thread Status:
Not open for further replies.

Share This Page