ConcurrentModificationException when using EntryTreeHashMap's iterator

d-edery commented 6 years ago

Related exception:

java.util.ConcurrentModificationException at java.util.TreeMap$PrivateEntryIterator.nextEntry(TreeMap.java:1232) at java.util.TreeMap$KeyIterator.next(TreeMap.java:1286) at net.jodah.expiringmap.ExpiringMap$EntryTreeHashMap$AbstractHashIterator.getNext(ExpiringMap.java:473) at net.jodah.expiringmap.ExpiringMap$EntryTreeHashMap$ValueIterator.next(ExpiringMap.java:497) ...

The code on which this exception is thrown does something like:

for (MyEntity myEntity : myCachedEntities.values()) {
    ...
}

The expiring map is built with the following config:

private ExpiringMap<String, MyEntity> myCachedEntities = ExpiringMap.builder()
    .variableExpiration()
    .asyncExpirationListener(new ExpirationListener<String, MyEntity>() {
        @Override
        public void expired(String key, MyEntity value) {
            ...
        }
    })
    .build();

I've hit this bug constantly in our testing scenarios (never in production, this is probably due to high rate of cache read/write/expiration from multiple threads).

The solution to the problem was to change the type of sortedSet in EntryTreeHashMap from TreeSet to ConcurrentSkipListSet (safe, implements the SortedSet interface)

  /** Entry TreeHashMap implementation for variable expiration ExpiringMap entries. */
  private static class EntryTreeHashMap<K, V> extends HashMap<K, ExpiringEntry<K, V>> implements EntryMap<K, V> {
    private static final long serialVersionUID = 1L;
    SortedSet<ExpiringEntry<K, V>> sortedSet = new ConcurrentSkipListSet<>();

A pull request is on its way (leaving the issue open until PR is approved & merged / other valid suggestions)

d-edery commented 6 years ago

ping. more than a month passed. no comment?