ConcurrentModificationException on single-threaded code

Question

Edit: Well, I feel rather sheepish. I was looking at the wrong constructor. The real constructor being called (see below) -was- violating the concurrency rules of the foreach loop, as per Kal's answer.

Thank you for your help regardless! It will still help me fix the actual error with the code.

all

I'm a pretty new Java Programmer, and am only just starting to get a basic handle on the language. I'm currently working with a dialogue participant system, but have first been trying to get our system's representations of Logical Terms up to spec. I'm almost done, but have run into the following Error:

Exception in thread "main" java.util.ConcurrentModificationException
at java.util.AbstractList$Itr.checkForComodification(AbstractList.java:372)
at java.util.AbstractList$Itr.next(AbstractList.java:343)
at com.Term.<init>(Term.java:97)
at com.Term.substituteVariables(Term.java:251)
at com.Term.substituteVariables(Term.java:247)
at com.Term.substituteVariables(Term.java:247)
at com.TermPredTestArch.main(TermPredTestArch.java:40)

The method in question, substituteVariables, is basically a copy constructor, with a slight modification: it takes in a map of bindings, and recursively iterates through the Term Object that it is called from, finding variables along the way and swapping them out for their instantiations. Bizarrely, it seemed to be working just last night when I left (though I did not test extensively), but now refuses to play nice; I had made no substantial modifications.

The relevant code is as follows (lines 232-252):

232  /** Returns a new Term with the appropriate bindings substituted */
233  public Term substituteVariables(Map<Variable, Symbol> bindings) {
234    ArrayList<Symbol> args    = this.getArgs();
235    ArrayList<Symbol> newArgs = new ArrayList<Symbol>();
236    Set<Variable> vars        = this.getVars();
237    Set<Variable> bindingKeys = bindings.keySet();
238    for(Symbol s: args) {
239      // if s is a Variable, check to see if it has a substituion, and
240      // if so, swap it out
241      if(s instanceof Variable) {
242        if(bindingKeys.contains(s)) newArgs.add(bindings.get(s));
243        else                        newArgs.add(s);
244      // if s is a Term, add it and recursively substitute any variables
245      // it has within the current set of bindings
246      } else if(s instanceof Term) {
247        newArgs.add(((Term) s).substituteVariables(bindings));
248      // if s is just a symbol, simply add it to the args
249      } else newArgs.add(s);
250    }
251    return new Term(this.getName(), newArgs);
252  }

EDIT: And here is the constructor for Term:

public Term(String n, ArrayList<Symbol> a) {
    super(n);
    args = a;
    HashSet<Variable> varsToAdd = new HashSet<Variable>();
    for(Symbol s: a) parseArg(s.toString());
}          

That is the -actual- constructor that was being called, not the one that I thought was being called. Which does, in fact, violate the foreach loop concurrency rules, as per Kal's answer.

From the research I've already done, I know that ConcurrentModificationException is often caused by multiple threads iterating/modifying a Collection simultaneously without synchronization, but I have no deliberate parallelism here, nor anywhere else in the class, or the test code that's using it. Otherwise, I'm not entirely sure. the javadoc for the class mentions that it could also be cause by an iterator simultaneously iterating and modifying a Collection, but I don't think I'm doing that either; I'm merely observing the iterated Collection and using information from it to build another Collection. Does that violate concurrency regulations as well?

Any pointers you all might be able to provide would be hugely appreciated! I will also pre-emptively apologise for any egregious breaches of Java etiquette or style (feel free to point those out too!).

Thanks

Answer 1

ConcurrentModificationException occurs when you modify an ArrayList while iterating through it using the for loop.

The way to do this correctly is to use the Iterators add/remove methods.

Here is the relevant documentation from the API --

The iterators returned by this class's iterator and listIterator methods are fail-fast: if list is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove or add methods, the iterator will throw a ConcurrentModificationException. Thus, in the face of concurrent modification, the iterator fails quickly and cleanly, rather than risking arbitrary, non-deterministic behavior at an undetermined time in the future.