+6  A: 

Calling remove on a collection while you're iterating through it will cause a ConcurrentModificationException every time, even if it's all done in the same thread - the Right Thing to do is get an explicit iterator and call .remove() on that.

Edit: Modifying your example:

Iterator<Entry<GenericEvent, Command>> i = eventMultiMap.entries.iterator();
while (i.hasNext()) {
    if (i.next().getValue().equals(command)) {
        i.remove();
        nbRemoved++;
    }
}
MHarris
Ok, i get the point. But Iterator<E> can only takes one parameter type. No ? If i iterate trought the values of my map, and remove one them. Will the corresponding value will deleted in the original map ( eventMultiMap ) ?
Antoine Claval
Ok, the iterator on the values of the map can remove the values. Thanks. I cannot accepts your response for now, Iterator<E,T> is not compiling, but edit it and i will accept.
Antoine Claval
Sorry. Don't have google-collections at work, so I couldn't test the code before posting it. If they're designed like the Java HashMap, it should be possible to use an Iterator over the Entry objects - I've edited to show this and will doublecheck it when I get home, if you've not had a chance to try it in the meantime.
MHarris
+3  A: 

If another thread could modify your multimap while this logic is running, you'll need to add a synchronized block to MHarris's code:

synchronized (eventMultimap) {
  Iterator<Entry<GenericEvent, Command>> i = eventMultiMap.entries.iterator();
  while (i.hasNext()) {
    if (i.next().getValue().equals(command)) {
        i.remove();
        nbRemoved++;
    }
  }
}

Or, you could omit the iterator as follows,

synchronized (eventMultimap) {
  int oldSize = eventMultimap.size();
  eventMultimap.values().removeAll(Collections.singleton(command));
  nbRemoved = oldSize - eventMultimap.size();
}

The removeAll() call doesn't require synchronization. However, if you omit the synchronized block, the multimap could mutate between the removeAll() call and one of the size() calls, leading to an incorrect value of nbRemoved.

Now, if your code is single-threaded, and you just want to avoid a ConcurrentModificationException call, you can leave out the Multimaps.synchronizedMultimap and synchronized (eventMultimap) logic.

Jared Levy
I like the removeAll(Collection.singleton(stuffToRemove)). Thanks.
Antoine Claval
A: 

You may wish to see this blogpost for another pitfall yielding a ConcurrentModificationException when traversing a multimap, with no other thread interfering. In short, if you traverse multimap's keys, accessing the respective collection of values associated with each key and remove some element from such a collection, if that element happens to be the last of the collection you are going to have ConcurrentModificationException when you try to access the next key - because emptying a collection triggers the removal of the key, thus structurally modifying the multimap's keyset.

Dimitris Andreou