ConcurrentModificationException: Remove during Loop - Java Common Bug Pattern


It's a common bug that we try to use collection.remove during looping the collection.

For example: in ehcache 2.7.0 getAll method
    public Map getAll(Collection keys) throws IllegalStateException, CacheException {
        getAllObserver.begin();
        checkStatus();

        if (disabled) {
            return null;
        }

        if (keys.isEmpty()) {
            return Collections.EMPTY_MAP; // optimize here: no need to create new map
        }

        Map elements = compoundStore.getAll(keys);

        long misses = 0;
        for (Entry entry : elements.entrySet()) {
            Object key = entry.getKey();
            Element element = entry.getValue();
            if (element == null) {
                misses++;
            } else {
                if (isExpired(element)) {
                    tryRemoveImmediately(key, true);
                    elements.remove(key); // bug here.
                    misses++;
                } else {
                    element.updateAccessStatistics();
                }
            }
        }
        int requests = elements.size();
        if (misses == 0) {
            getAllObserver.end(GetAllOutcome.ALL_HIT, requests, 0);
        } else if (misses == requests) {
            getAllObserver.end(GetAllOutcome.ALL_MISS, 0, misses);
        } else {
            getAllObserver.end(GetAllOutcome.PARTIAL, requests - misses, misses);
        }
        return elements;
    }

This was fixed in latest version: ehcache 2.10.0 getAll method
Usually we use Iterator> it=map.entrySet().iterator(); then use it.remove() to delete the element.
Here the iterator of the map returned by compoundStore.getAll() may not support remove method. 
That's why ehcache first adds all expired key to a HashSet, then call:
          try {
              elements.keySet().removeAll(expired);
          } catch (UnsupportedOperationException e) {
              elements = new HashMap(elements);
              elements.keySet().removeAll(expired);
          }

Here is the HashSet removeAll method:
     * Note that this implementation will throw an
     * UnsupportedOperationExceptionif the iterator returned by the

     * iteratormethod does not implement the removemethod.
    public boolean removeAll(Collection c) {
        Objects.requireNonNull(c);
        boolean modified = false;

        if (size() > c.size()) { // optimization: different branch based on size diff
            for (Iterator i = c.iterator(); i.hasNext(); )
                modified |= remove(i.next());
        } else {
            for (Iterator i = iterator(); i.hasNext(); ) {
                if (c.contains(i.next())) {
                    i.remove();
                    modified = true;
                }
            }
        }
        return modified;

    }
ehcache 2.10.0 getAll method
public Map getAll(Collection keys) throws IllegalStateException, CacheException {
        getAllObserver.begin();
        checkStatus();

        if (disabled) {
            return null;
        }

        if (keys.isEmpty()) {
            getAllObserver.end(GetAllOutcome.ALL_HIT, 0, 0);
            return Collections.EMPTY_MAP;
        }

        Map elements = compoundStore.getAll(keys);
        Set expired = new HashSet();
        for (Entry entry : elements.entrySet()) {
            Object key = entry.getKey();
            Element element = entry.getValue();
            if (element != null) {
                if (isExpired(element)) {
                    tryRemoveImmediately(key, true);
                    expired.add(key);
                } else {
                    element.updateAccessStatistics();
                }
            }
        }
        if (!expired.isEmpty()) {
          try {
              elements.keySet().removeAll(expired);
          } catch (UnsupportedOperationException e) {
              elements = new HashMap(elements);
              elements.keySet().removeAll(expired);
          }
        }

        int requests = keys.size();
        int hits = elements.size();
        if (hits == 0) {
            getAllObserver.end(GetAllOutcome.ALL_MISS, 0, requests);
        } else if (requests == hits) {
            getAllObserver.end(GetAllOutcome.ALL_HIT, requests, 0);
        } else {
            getAllObserver.end(GetAllOutcome.PARTIAL, hits, requests - hits);
        }
        return elements;
    }

Labels

adsense (5) Algorithm (69) Algorithm Series (35) Android (7) ANT (6) bat (8) Big Data (7) Blogger (14) Bugs (6) Cache (5) Chrome (19) Code Example (29) Code Quality (7) Coding Skills (5) Database (7) Debug (16) Design (5) Dev Tips (63) Eclipse (32) Git (5) Google (33) Guava (7) How to (9) Http Client (8) IDE (7) Interview (88) J2EE (13) J2SE (49) Java (186) JavaScript (27) JSON (7) Learning code (9) Lesson Learned (6) Linux (26) Lucene-Solr (112) Mac (10) Maven (8) Network (9) Nutch2 (18) Performance (9) PowerShell (11) Problem Solving (11) Programmer Skills (6) regex (5) Scala (6) Security (9) Soft Skills (38) Spring (22) System Design (11) Testing (7) Text Mining (14) Tips (17) Tools (24) Troubleshooting (29) UIMA (9) Web Development (19) Windows (21) xml (5)