Solved ConcurrentModificationException with Iterator & HashMap

Discussion in 'Spigot Plugin Development' started by AuroraLS3, Jan 18, 2017.

  1. Hello, got this error while iterating over the keyset of my cache hashmap.

    Code (Text):
    [11:36:08]
    [11:36:08] [Craft Scheduler Thread - 665/WARN]: [Plan] Plugin Plan v2.0.0 generated an exception while executing task 24
    java.util.ConcurrentModificationException
    at java.util.HashMap$HashIterator.nextNode(Unknown Source) ~[?:1.8.0_77]
    at java.util.HashMap$KeyIterator.next(Unknown Source) ~[?:1.8.0_77]
    at com.djrapitops.plan.data.cache.DataCacheHandler.clearCache(DataCacheHandler.java:225) ~[?:?]
    at com.djrapitops.plan.data.cache.DataCacheHandler$1.run(DataCacheHandler.java:83) ~[?:?]
    at org.bukkit.craftbukkit.v1_11_R1.scheduler.CraftTask.run(CraftTask.java:71) ~[spigot_latest.jar:git-Spigot-0b1090d-7fdc749]
    at org.bukkit.craftbukkit.v1_11_R1.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:52) [spigot_latest.jar:git-Spigot-0b1090d-7fdc749]
    at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source) [?:1.8.0_77]
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source) [?:1.8.0_77]
    at java.lang.Thread.run(Unknown Source) [?:1.8.0_77]


    The Code responsible for the error is here:

    Code (Text):
        /**
         * Clears all UserData from the HashMap
         */
        public void clearCache() {
            Set<UUID> uuidSet = dataCache.keySet();
            Iterator<UUID> uuidIterator = uuidSet.iterator();
            while (uuidIterator.hasNext()) {
                clearFromCache(uuidIterator.next()); <<<< Line 225
            }
        }

        /**
         * Clears the matching UserData from the HashMap
         *
         * @param uuid Player's UUID
         */
        public void clearFromCache(UUID uuid) {
            if (dataCache.get(uuid) != null) {
                if (dataCache.get(uuid).isAccessed()) { // Check if the data is being saved.
                    (new BukkitRunnable() {
                        @Override
                        public void run() {
                            if (!dataCache.get(uuid).isAccessed()) { // Check if the data is being saved.
                                dataCache.remove(uuid);
                                plugin.log("Cleared " + uuid.toString() + " from Cache. (Delay task)");
                                this.cancel();
                            }
                        }
                    }).runTaskTimer(plugin, 30 * 20, 30 * 20);
                } else {
                    dataCache.remove(uuid);
                    plugin.log("Cleared " + uuid.toString() + " from Cache.");
                }
            }
        }
    This piece of code is called only after X number of saves, and it is called after a save, that is done in an asynchronous task.
    How can I clear every value from the memory in the HashMap without getting ConcurrentModificationException (and preventing data that is being saved from being erased)?
    Should I just add a delay to the clear after the save (with a runTaskLater)?

    Thanks,
    -Rsl
     
  2. You can't remove/add from a map or collection while iterating it. Use Iterator#remove.
     
  3. That would only remove it from the keySet, no?
     
  4. The key set and values collection are representive views of the map, if you manipulate those collections it'll reflect in the map as well. (e.g. a #remove from the keyset removes from the whole map).
     
  5. Is the problem solved? :)
     
  6. Not yet since I'm trying to figure out how to pass the iterator to the method below while retaining the correct entry to iterate
     
  7. Most likely solved with this solution:

    Code (Text):
        public void clearCache() {
            Iterator<Map.Entry<UUID, UserData>> clearIterator = dataCache.entrySet().iterator();
            while (clearIterator.hasNext()) {
                clearFromCache(clearIterator.next());
            }
        }

        public void clearFromCache(Map.Entry<UUID, UserData> entry) {      
                if (entry.getValue().isAccessed()) {
                    (new BukkitRunnable() {
                        @Override
                        public void run() {
                            if (entry.getValue().isAccessed()) {
                                entry.setValue(null);
                                plugin.log("Cleared " + entry.getKey().toString() + " from Cache. (Delay task)");
                                this.cancel();
                            }
                        }
                    }).runTaskTimer(plugin, 30 * 20, 30 * 20);
                } else {
                    entry.setValue(null);
                    plugin.log("Cleared " + entry.getKey().toString() + " from Cache.");
                }
        }
     
  8. Setting the value to null will not remove it from the map.

    As I said, you need Iterator#remove to be called. You can either do that via returning a boolean to determine if a removal is needed (and check against that to call #remove yourself), or you can pass the iterator to the method.
     
  9. I know that it does not remove it from the HashMap.
    It replaces the reference though, which means that the old Object that was stored should be cleared by the garbage collector.
    I have null checks for the getters of this key anyway, so this should clear the stuff from RAM without further complications.
    I shall use the iterator in another timed task to get rid of the nulls.
     
  10. You'll still have an entry in the map, which will not be garbage collected. The UUID will also remain in the map.

    Why set to null when you can literally just remove the unneccessary references?
     
  11. Because I'm not fluent enough with iterators to know what entry it is using with iterator.remove(). Does it move to the next entry when I iterator.remove() is called or is another next() needed?
     
  12. It will remove whatever value you retrieved from Iterator#next.

    You can find Iterator's documentation here: https://docs.oracle.com/javase/8/docs/api/java/util/Iterator.html