Search code examples
javalistsublist

How to deal with ConcurrentModificationException in this case


I'm working on a contour scanner. For every contour I want to save the corners / edge coördinates. Instead of having an Array in the contour I have 1 big Array that I share. The reason for this is that it has to work on animations so this is for optimization.

The idea is that every contour get's a subList view of the big array.

I worked quite long on a class to make that easy and now I run into a problem:

ArrayList<PVector> vecs = new ArrayList<PVector>();

for (int i = 0; i < 10; i++) {
  vecs.add(new PVector());
}

List<PVector> subList = vecs.subList(0, 5);

for (int i = 0; i < 10; i++) {
  vecs.add(new PVector());
}

// ConcurrentModificationException
for (int i = 0; i < subList.size (); i++) {

}

First, is it poor java design that it does throws the concurrent modification if I use add(Object)? This should have no influence on any subList I already have since it adds to the end right? (logical speaking). My point is, add(Object) can never influence an already excisting subList, only add(index, Object) can do that (and other things like remove, swap and sort).

Second. What would be a good way to deal with this? I could make the big array really big so it's less likely I need to add elements after I already made a subList but I wonder if there are any other good ways to deal with it.

Here is a class I made that had to make it easy for me but is making my life hard right now :)

public class ListDivisor<T> {

    List<T> list;

    int subListStartIndex = 0;
    int currentGetIndex = 0;

    InstanceHelper instanceHelper;


    public ListDivisor(List<T> list, InstanceHelper<T> instanceHelper) {
        this.list = list;
        this.instanceHelper = instanceHelper;
    }

    // . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .


    public void reset() {
        subListStartIndex = 0;
        currentGetIndex = 0;

        if (instanceHelper.doResetInstances()) {
            for (T obj : list) {
                instanceHelper.resetInstance(obj);
            }
        }
    }

    // . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .


    public List<T> getSubList(int size) {

        int fromIndex = subListStartIndex; // inclusive
        int toIndex = fromIndex + size; // exclusive

        for (int i = list.size(); i < toIndex; i++) {
            list.add((T) instanceHelper.createInstance());
        }

        subListStartIndex = toIndex;
        currentGetIndex = toIndex;

        return list.subList(fromIndex, toIndex);
    }

    // . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .

    /**
     * Returns a subList starting where the previous subList ended till
     * the latest object added till then.
     *
     * @return
     */
    public List<T> getSubList() {
        return getSubList(currentGetIndex-subListStartIndex);
    }

    // . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .


    public T getNext() {

        if (currentGetIndex >= list.size()) {
           list.add((T)instanceHelper.createInstance());
        }

        return list.get(currentGetIndex++);

    }

    // . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .


    public void clear() {
        list.clear();
        reset();
    }


    // . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .

    public interface InstanceHelper<T> {
        public T createInstance();
        public boolean doResetInstances();
        public void resetInstance(T obj);
    }

}

This is a small example of how to use the class:

 ListDivisor<PVector> vectorsDivisor = new ListDivisor<PVector>(
     new ArrayList<PVector>(), 
     new InstanceHelper<PVector>() {
            //@Override
            public PVector createInstance() {
                return new PVector();
            }

            //@Override
            public boolean doResetInstances() {
              return true;
            }

            //@Override
            public void resetInstance(PVector v) {
                 v.set(0,0,0);              
            }
     });

  PVector v = vectorsDivisor.getNext();
  v.set(1, 1, 1);

  v = vectorsDivisor.getNext();
  v.set(2, 2, 2);

  v = vectorsDivisor.getNext();
  v.set(3, 3, 3);

  subList = vectorsDivisor.getSubList();


  println("subList size: "+subList.size());

Solution

  • According to the javadocs for ArrayList.sublist:

    "The semantics of the list returned by this method become undefined if the backing list (i.e., this list) is structurally modified in any way other than via the returned list. (Structural modifications are those that change the size of this list, or otherwise perturb it in such a fashion that iterations in progress may yield incorrect results.)"

    In the implementation of ArrayList in the JVM that you are using, the "unspecified" semantics would appear to be that it throws a ConcurrentModificationException.


    First, is it poor java design that it does throws the concurrent modification if I use add(Object)?

    Nope. It is not "poor design".

    They designed that way, deliberately, and for good reasons. Furthermore, they documented that you would get unspecified behaviour if you did what you are doing. And indeed, they have implemented the sublist class to fast fail if it detects a concurrent modification. That is a good thing ... and entirely consistent with the API spec.

    This should have no influence on any subList I already have since it adds to the end right?

    The problem is that while your example is (may be) safe, others are not. And it is not possible to distinguish the safe and unsafe cases without adding a whole lot of additional infrastructure that would most likely make the ArrayList less efficient and more memory intensive for other use-cases.

    (Hence my "for good reason" comment above.)

    Second. What would be a good way to deal with this?

    It is hard to advise on that without understanding what your code actually needs to do. But some generic alternatives spring to mind:

    • Change your algorithm so that you don't use the List.sublist method; e.g. pass the first and last indexes of your notional sublists as parameters.

    • Recreate the sublist object each time you update the backing list. (Assuming that your updates to the backing list are really safe, then this is simple, and trivially correct ... assuming that you are not in the middle of using the sublist's iterator.)

    • Change your code to use a Queue or Deque instead of a List.

    • Implement your own List class (based on ArrayList) that doesn't throw CME's in this safe scenario.