0

I'm getting occasionally ConcurrentModificationException with the following code:

public Set<MyObject> getTypes(Set<Type> names) {
        Set<MyObject> myObjects = new HashSet<>();          
        myObjects = names.stream().filter(Objects::nonNull).mapToInt(Type::getId)
                .mapToObj(cache::getMyObject)
                .collect(Collectors.toSet());

I'm using cache to convert to MyObject, but exception seems to thrown in collect method (DAOImpl.java line 114)

java.util.ConcurrentModificationException
        at java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1558)
        at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
        at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
        at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
        at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
        at com.dao.DAOImpl.getTypes(DAOImpl.java:114)
        at com.dao.DAOImpl$$FastClassBySpringCGLIB$$affe23c4.invoke(<generated>)
        at org.springframework.cglib.proxy.MethodProxy.invoke(MethodProxy.java:218)
        at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.invokeJoinpoint(CglibAopProxy.java:779)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
        at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:750)
        at org.springframework.dao.support.PersistenceExceptionTranslationInterceptor.invoke(PersistenceExceptionTranslationInterceptor.java:137)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
        at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:750)
        at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:692)
        at com.dao.DAOImpl$$EnhancerBySpringCGLIB$$6000bd7e.getTypes(<generated>)

How can I use cache to map to objects, or must I use cache outside stream?

Notice cache should be updated only 1 per day

Ori Marko
  • 56,308
  • 23
  • 131
  • 233
  • 1
    If you get a `ConcurrentModificationException` it means that something is modifying `names` while streaming its entries. Is your `cache` somehow related to it? What else is modifying that collection? – Didier L Feb 14 '22 at 11:46
  • Is your `cache` a plain `HashMap` by any chance? – Kayaman Feb 14 '22 at 11:50
  • @Kayaman cache is component which uses `Map` implemented by `HashMap` to get the object by id – Ori Marko Feb 14 '22 at 12:00
  • @DidierL `cache` isn't updating `names` and I don't see `names` modified – Ori Marko Feb 14 '22 at 12:04
  • Is your cache thread-safe? I've never seen a proper cache implemented with a regular `HashMap`, there are obvious concurrency issues with it. `ConcurrentHashMap` can't throw a `CME` and it can be used as a simple cache as-is. – Kayaman Feb 14 '22 at 12:06
  • @Kayaman you may be correct, but the specific cache is daily basis, so it shouldn't be updated normally – Ori Marko Feb 14 '22 at 12:11
  • 1
    Yet it's still being modified, as the exception shows. You've got a bug somewhere, whether a small bug in your caching, or a larger logic error in your daily caching. – Kayaman Feb 14 '22 at 12:14
  • I think this question really needs a [mre] because currently we can’t understand what’s going on. – Didier L Feb 14 '22 at 12:49
  • (I am also a bit confused by the stacktrace because the calls between `HashMap$KeySpliterator.forEachRemaining()` and `DAOImpl.getTypes()` make it look like if you were calling directly `names.stream().collect()` without any intermediate operation – but maybe I’m just missing something) – Didier L Feb 14 '22 at 13:02

2 Answers2

0

The problem is not related with your cache::getMyObject. The problem is that your names Set is being changed while you stream it, hence the ConcurrentModificationException.

If you wrap (create a new collection) from your names collection the problem is fixed.

public Set<MyObject> getTypes(Set<Type> names) {
        Set<MyObject> myObjects = new HashSet<>(names).stream().filter(Objects::nonNull).mapToInt(Type::getId)
                .mapToObj(cache::getMyObject).collect(Collectors.toSet());

Nonetheless, I think your cache method (getMyObject) can return null. In that case you should filter them out.

If you want to trigger the exception you can do something like this:

 public static void main(String[] args) {
    Set<String> names = new HashSet();

    names.add("a");
    names.add("b");

    Thread x = new Thread(){
      @Override
      public void run() {
        try {
          sleep(1000);
        } catch (InterruptedException e) {
          e.printStackTrace();
        }

        names.add("c");
      }
    };

    x.start();

    Set<Object> myObjects = new HashSet<>(names).stream().filter(Objects::nonNull).mapToInt(s -> s.hashCode()).mapToObj(i -> String.valueOf(i)).map(x1-> getx(x1)).collect(Collectors.toSet());

  }

  private static Object getx(String x) {

    try {
      Thread.sleep(2000);
    } catch (InterruptedException e) {
      e.printStackTrace();
    }

    return x;
  }
pringi
  • 3,987
  • 5
  • 35
  • 45
  • 1
    Depending on what is the origin of the concurrent modification, `new HashSet<>(names)` could also throw a CME. – Didier L Feb 14 '22 at 11:55
  • Why Set is being modified ? I don't modified it. do you know how can test your fix? because it isn't reproducible (happens every X hours) – Ori Marko Feb 14 '22 at 12:04
  • Well, From the code you posted the most obvious explanation is names being changed. But there is a line in the exception that you posted com.dao.DAOImpl.getTypes(DAOImpl.java:114 that now has called for my attention. If you say that you do not modify names, then what is cache:getMyObject and what is line 114 of DAOImpl? – pringi Feb 14 '22 at 12:37
  • @pringi line 114 is `.collect(Collectors.toSet());` in the code shown – Ori Marko Feb 14 '22 at 13:27
0

The issue was that 2 threads changed myObjects so the expected count of the Set when looping through it was compromised, therefore the exception

The fix was to make the above service call atomic and therefore to prevent changing Set on runtime with a different thread

Ori Marko
  • 56,308
  • 23
  • 131
  • 233