1

During recent load test, we caught java.util.ConcurrentModificationException at several GET REST endpoints. The exception stack trace shows that exception is thrown during foreach loop over collection of objects:

java.base/java.util.ArrayList$Itr.checkForComodification(Unknown Source)
    at  java.base/java.util.ArrayList$Itr.next(Unknown Source)
    at  com.myapp.service.ApplicantBioServiceImpl.getOtherInfo(ApplicantBioServiceImpl.java:1497)
    at  jdk.internal.reflect.GeneratedMethodAccessor1214.invoke(Unknown Source)
    at  java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
    at  java.base/java.lang.reflect.Method.invoke(Unknown Source)
    at  org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:343)
    at  org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:198)
    at  org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:163)
    at  org.springframework.transaction.interceptor.TransactionInterceptor$$Lambda$965/0x0000000000000000.proceedWithInvocation(Unknown Source)
    at  org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:295)
    at  org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:98)
    at  org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
    at  org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:212)
    at  com.sun.proxy.$Proxy788.getOtherInfoUX(Unknown Source)

The method looks like this

@Override
@Transactional
public AbstractBlockDTO getOtherInfo(long userId, long sectionId) {
    AbstractBlockDTO blockDTO = questionService.getQuestionBlockData(userId, sectionId);
    ServiceUtility.sortBlock(blockDTO);
    User user = getUser(userId);

    for (AbstractDataDTO dataDTO : blockDTO.getDataDTO()) {
        String questionBlockName = dataDTO.getId();

        if (questionBlockName.equalsIgnoreCase(Constants.ETHNICITY)) {
            populateEthnicityInfo(dataDTO, user, instanceId);
        } else if (questionBlockName.equalsIgnoreCase(Constants.RACE)) {
            populateRaceInfo(dataDTO, user, instanceId);
        } else if (questionBlockName.equalsIgnoreCase(Constants.ETHNICITY_2)) {
                populateEthnicity2(dataDTO, user, instanceId);
        }
    }
    return blockDTO;
}

As you can see, there is no logic that modifies(add/remove) the collection within the foreach loop. The AbstractBlockDTO class looks like this:

public class AbstractBlockDTO implements Serializable {
    private static final long serialVersionUID = -2718327468035422694L;

    // other fields
    
    @JsonProperty("questionBlocks")
    private List<AbstractDataDTO> dataDTO = new ArrayList<>();

public List<AbstractDataDTO> getDataDTO() {
        return dataDTO;
    }

    public void setDataDTO(List<AbstractDataDTO> dataDTO) {
        this.dataDTO = dataDTO;
    }

    // Builder
}

Assumptions: The questionService.getQuestionBlockData(userId, sectionId); returns object that is fetched from cache. For caching we are using Spring Caching with a memcashed. My initial thought was that cache returns the same obj instance, therefore we may end up in the situation when the collection is sorted in one thread and iterated in another one. But I can't reproduce it with the integration test. In the app logs I see the different objects are returned for the same key, but I am not sure if it just different references that point to the same object.

CUSTOM_QUESTIONS6789 from cache configCache value 'com.myapp.beans.AbstractBlockDTO@72c90d72'
CUSTOM_QUESTIONS6789 from cache configCache value 'com.myapp.beans.AbstractBlockDTO@531640e1'

I struggle with this issue and appreciate any input that will help me to understand why the exception happening and how to solve it, thanks.

Hutsul
  • 1,535
  • 4
  • 31
  • 51
  • 2
    Does this answer your question? [Why is a ConcurrentModificationException thrown and how to debug it](https://stackoverflow.com/questions/602636/why-is-a-concurrentmodificationexception-thrown-and-how-to-debug-it) – vaibhavsahu Aug 30 '21 at 21:35
  • @vaibhavsahu, thanks for response. But it doesn't help me. In the suggested answer, the assumption is that collection is modified by removing elements from it which is happening within the `for` loop. In my case, I am not adding or removing any elements while iterating over collection. – Hutsul Aug 30 '21 at 21:42
  • 1
    *My initial thought was that cache returns the same obj instance, therefore we may end up in the situation when the collection is sorted in one thread and iterated in another one.* That seems likely. *I am not sure if it just different references that point to the same object.* Why don't you check? – shmosel Aug 30 '21 at 21:46
  • @shmosel, thanks for response. Is there any easy way I can check that while running integration test? Or `DEBUG` is the only way? – Hutsul Aug 30 '21 at 21:52
  • Just `if(x == y)`. – Boris the Spider Aug 30 '21 at 22:04
  • @shmosel, I've verified my assumption. I invoked the service that returns data from cache two times in the same thread and made the reference comparison `abstractBlockDTO == abstractBlockDTO2`. The result is `false`. Now, I don't have any ideas about why the issue may happening. Any suggestions? – Hutsul Aug 30 '21 at 23:15
  • 1
    Handing out a reference to a mutable object implies losing control over it. Are such modifications intended or should `setDataDTO` be used for all modifications? In the latter case, use `public void setDataDTO(List dataDTO) { this.dataDTO = Collections.unmodifiableList(dataDTO); }` or even make a defensive copy, `public void setDataDTO(List dataDTO) { this.dataDTO = Collections.unmodifiableList(new ArrayList<>(dataDTO)); }`. Then, check where unintended modifications bail out. – Holger Aug 31 '21 at 11:51

0 Answers0