Concurrent Modification Exception

[joerandazzo]

Hello,

I noticed an older issue reporting ConcurrentModificationException, and found the issue is still occurring in 0.4.2. I tried keyset and entrySet.

    java.util.ConcurrentModificationException
            at java.util.LinkedHashMap$LinkedHashIterator.nextEntry(LinkedHashMap.java:350)
            at java.util.LinkedHashMap$EntryIterator.next(LinkedHashMap.java:379)
            at java.util.LinkedHashMap$EntryIterator.next(LinkedHashMap.java:377)
            at net.jodah.expiringmap.ExpiringMap$EntryLinkedHashMap$AbstractHashIterator.getNext(ExpiringMap.java:293)
            at net.jodah.expiringmap.ExpiringMap$EntryLinkedHashMap$EntryIterator.getNext(ExpiringMap.java:314)
            at net.jodah.expiringmap.ExpiringMap$EntryLinkedHashMap$EntryIterator.next(ExpiringMap.java:316)
            at net.jodah.expiringmap.ExpiringMap$EntryLinkedHashMap$EntryIterator.next(ExpiringMap.java:314)

[joerandazzo]

And to be clearer, this is relating to setting the expiration policy to ACCESSED

[jhalterman]

Edit: If you do a map.get while iterating on a keySet with ExpirationPolicy.ACCESSED you will get ConcurrentModificationException since the get can effectively modify the map by removing expired elements. Instead, use entrySet:

for (Map.Entry<String, String> entry : map.entrySet()) {
  System.out.printf("key:" + entry.getKey() + " val:" + entry.getValue());
}

Let me know if you still have problems.

[MohdArshil]

Hello @jhalterman ,
Facing same error periodically in the following implementation.
Map<String, Object> map = ExpiringMap.builder().expiration(3600, TimeUnit.SECONDS).build(); List<Object> objList = new ArrayList<>(map.values());

Any Suggestion for the solution.

[jhalterman]

@MohdArshil Are you iterating over keys while attempting to modify the map? Or are you doing some other operations?

[egg82]

ConcurrentMap provides two contracts:

1. Thread safety, meaning actions from one thread don't interfere with actions on another thread.
2. Atomicity, meaning all actions are performed as a single operation (eg. passing a lambda with several operations into a compute method is guaranteed to be performed as one operation, performed before or after another operation and not at the same time)

By throwing a ConcurrentModificationException on modification of a ConcurrentMap while iterating, you've broken the first contract. Broken Java contracts are why I have trust issues.

All joking aside, it would definitely be appreciated if we were able to modify (or iterate with expired values, in the case of #30 ) without getting unexpected exceptions. It was quite strange seeing a ConcurrentModificationException being thrown from a ConcurrentMap.

I've only glanced at some of the code and errors, but it looks like a lock or map copy may be needed when iterating. Possibly an expired value check before an iterator is given?

[jhalterman]

ExpiringMap doesn't implement the iterator, it obtains it from an underlying data structure, so it doesn't have the opportunity to coordinate writes to the map with iteration. So we'd possibly need to create and control our own iterator.

Happy to review a PR for this.

[egg82]

Of course! I'll take a closer look at the codebase and PR if I can get some time.

[bratkartoffel]

Woah, sorry for the push spam, didn't expect github to write every single one here...

[jhalterman]

This has been fixed in 0.5.11.