views:

1302

answers:

5

I encountered ConcurrentModificationException and by looking at it I can't see the reason why it's happening; the area throwing the exception and all the places modifying the collection are surrounded by

synchronized (this.locks.get(id)) {
  ...
} // locks is a HashMap<String, Object>;

I tried to catch the the pesky thread but all I could nail (by setting a breakpoint in the exception) is that the throwing thread owns the monitor while the other thread (there are two threads in the program) sleeps.


How should I proceed? What do you usually do when you encounter similar threading issues?

+8  A: 

It may have nothing to do with the synchronization block. ConcurrentModificationExceptions often occur when you're modifying a collection while you are iterating over it's elements.

List<String> messages = ...;
for (String message : messages) {
    messages.add("A COMPLETELY NEW MESSAGE"); // Prone to ConcurrentModificationException
}
Adam Paynter
+1  A: 

It's common to receive a ConcurrentModificationException when modifying a dynamic list while iterating over it (in a foreach-loop for example). You may want to make sure you're not doing that anywhere.

Lucas Lindström
+4  A: 

Similar to a previous post, you can get the same issue if you delete an entry. e.g.

for(String message : messages) {
  if (condition(message))
     messages.remove(message);
}

Another common example is cleaning up a Map.

This particular problem can be resolved using an Iterator explicitly.

for(Iterator<String> iter = messages.iterator(); iter.hasNext();) {
   String message = iter.next();
   if (condition(message))
       iter.remove(); // doesn't cause a ConcurrentModificationException 
}
Peter Lawrey
+1  A: 

if you need to delete few elements from your list. You can maintain another list like elements to be removed. And finally call removeAll(collection). Of course this is not good for huge data.

iftee
A: 

Sometime your application may be complex too complex and some functions may have too much side effect. Also, maybe another thread is really doing something wrong with that list and you can't find where easily.

For my own problem, I've write my own list system that delegates another list and, once locked, all other modifications throws ConcurrentModificationException, so the bad modification instruction will get at output with the exception. It can also detect errors described above.

import java.util.*;

/**
 * Created by IntelliJ IDEA.
 * User: francoiscassistat
 * Date: 12 juin 2010
 * Time: 18:20:18
 *
 *
 * Lockable list, made to debug ConcurrentModificationException on Lists.
 * The lock can be switched on/off with setLocked(boolean).
 * When locked, all write access to the list or iterators gets ConcurrentModificationException.
 * Simple usage case :
 *
 * list.setLocked(true);
 *
 * for (Object o : list.iterator()) // now this won't get ConcurrentModificationException, the other instruction that cause this will thrown the exception
 * { ... }
 *
 * list.setLocked(false);
 */
public class LockableList<E> implements List<E> {
    protected class LockableListIterator implements Iterator<E> {
        protected Iterator<E> iterator;

        public LockableListIterator(Iterator<E> iterator) {
            this.iterator = iterator;
        }

        public boolean hasNext() {
            return iterator.hasNext();
        }

        public E next() {
            return iterator.next();
        }

        public void remove() {
            checkLock();
            iterator.remove();
        }
    }

    protected class LockableListListIterator implements ListIterator<E> {
        protected ListIterator<E> listIterator;

        public LockableListListIterator(ListIterator<E> listIterator) {
            this.listIterator = listIterator;
        }

        public boolean hasNext() {
            return listIterator.hasNext();
        }

        public E next() {
            return listIterator.next();
        }

        public boolean hasPrevious() {
            return listIterator.hasPrevious();
        }

        public E previous() {
            return listIterator.previous();
        }

        public int nextIndex() {
            return listIterator.nextIndex();
        }

        public int previousIndex() {
            return listIterator.previousIndex();
        }

        public void remove() {
            checkLock();
            listIterator.remove();
        }

        public void set(E e) {
            checkLock();
            listIterator.set(e);
        }

        public void add(E e) {
            checkLock();
            listIterator.add(e);
        }
    }

    protected class LockableListSubList implements List<E>
    {
        protected List<E> list;

        public LockableListSubList(List<E> list) {
            this.list = list;
        }

        public int size() {
            return list.size();
        }

        public boolean isEmpty() {
            return list.isEmpty();
        }

        public boolean contains(Object o) {
            return list.contains(o);
        }

        public Iterator<E> iterator() {
            return new LockableListIterator(list.iterator());
        }

        public Object[] toArray() {
            return list.toArray();
        }

        public <T> T[] toArray(T[] a) {
            return list.toArray(a);
        }

        public boolean add(E e) {
            checkLock();
            return list.add(e);
        }

        public boolean remove(Object o) {
            checkLock();
            return list.remove(o);
        }

        public boolean containsAll(Collection<?> c) {
            return list.containsAll(c);
        }

        public boolean addAll(Collection<? extends E> c) {
            checkLock();
            return list.addAll(c);
        }

        public boolean addAll(int index, Collection<? extends E> c) {
            checkLock();
            return list.addAll(index, c);
        }

        public boolean removeAll(Collection<?> c) {
            checkLock();
            return list.removeAll(c);
        }

        public boolean retainAll(Collection<?> c) {
            checkLock();
            return list.retainAll(c);
        }

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

        @Override
        public boolean equals(Object o) {
            return list.equals(o);
        }

        @Override
        public int hashCode() {
            return list.hashCode();
        }

        public E get(int index) {
            return list.get(index);
        }

        public E set(int index, E element) {
            checkLock();
            return list.set(index, element);
        }

        public void add(int index, E element) {
            checkLock();
            list.add(index, element);
        }

        public E remove(int index) {
            checkLock();
            return list.remove(index);
        }

        public int indexOf(Object o) {
            return list.indexOf(o);
        }

        public int lastIndexOf(Object o) {
            return list.lastIndexOf(o);
        }

        public ListIterator<E> listIterator() {
            return new LockableListListIterator(list.listIterator());
        }

        public ListIterator<E> listIterator(int index) {
            return new LockableListListIterator(list.listIterator(index));
        }

        public List<E> subList(int fromIndex, int toIndex) {
            return new LockableListSubList(list.subList(fromIndex, toIndex));
        }
    }

    protected List<E> list;
    protected boolean locked;

    public LockableList(List<E> list) {
        this.list = list;
        locked = false;
    }

    public boolean isLocked() {
        return locked;
    }

    public void setLocked(boolean locked) {
        this.locked = locked;
    }

    protected void checkLock() {
        if (locked)
            throw new ConcurrentModificationException("Locked");
    }

    public int size() {
        return list.size();
    }

    public boolean isEmpty() {
        return list.isEmpty();
    }

    public boolean contains(Object o) {
        return list.contains(o);
    }

    public Iterator<E> iterator() {
        return new LockableListIterator(list.iterator());
    }

    public Object[] toArray() {
        return list.toArray();
    }

    public <T> T[] toArray(T[] a) {
        return list.toArray(a);
    }

    public boolean add(E e) {
        checkLock();
        return list.add(e);
    }

    public boolean remove(Object o) {
        checkLock();
        return list.remove(o);
    }

    public boolean containsAll(Collection<?> c) {
        return list.containsAll(c);
    }

    public boolean addAll(Collection<? extends E> c) {
        checkLock();
        return list.addAll(c);
    }

    public boolean addAll(int index, Collection<? extends E> c) {
        checkLock();
        return list.addAll(index, c);
    }

    public boolean removeAll(Collection<?> c) {
        checkLock();
        return list.removeAll(c);
    }

    public boolean retainAll(Collection<?> c) {
        checkLock();
        return list.retainAll(c);
    }

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

    @Override
    public boolean equals(Object o) {
        return list.equals(o);
    }

    @Override
    public int hashCode() {
        return list.hashCode();
    }

    public E get(int index) {
        return list.get(index);
    }

    public E set(int index, E element) {
        checkLock();
        return list.set(index, element);
    }

    public void add(int index, E element) {
        checkLock();
        list.add(index, element);
    }

    public E remove(int index) {
        checkLock();
        return list.remove(index);
    }

    public int indexOf(Object o) {
        return list.indexOf(o);
    }

    public int lastIndexOf(Object o) {
        return list.lastIndexOf(o);
    }

    public ListIterator<E> listIterator() {
        return new LockableListListIterator(list.listIterator());
    }

    public ListIterator<E> listIterator(int index) {
        return new LockableListListIterator(list.listIterator(index));
    }

    public List<E> subList(int fromIndex, int toIndex) {
        return new LockableListSubList(list.subList(fromIndex, toIndex));
    }
}

Simply use it like this :

List list = new LockableList(new ArrayList(...));
list.setLocked(true);

for (E e : list.iterator())
{ ... }

list.setLocked(false);

Hope it may help someone else.

François Cassistat