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

I think you need to show the Term constructor since that's what's throwing it. — Anon
– Anon, Commented Jul 7, 2011 at 17:45
What does the constructor of Term look like? That's where the exception is being thrown. — Jim Garrison
– Jim Garrison, Commented Jul 7, 2011 at 17:46
@Derek: If you see some answer answers your question, press the accept tick on the side of it. The answerer gets his reputation :) — mico
– mico, Commented Jul 7, 2011 at 19:04

Kal · Accepted Answer · 2011-07-07 17:51:51Z

14

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.

edited Jul 7, 2011 at 17:51

answered Jul 7, 2011 at 17:45

Kal

24.9k8 gold badges67 silver badges66 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

mre Over a year ago

Iterator doesn't have an add(...) method, although ListIterator does.

Grambot · Accepted Answer · 2011-07-07 17:49:19Z

0

newArgs.add(((Term) s).substituteVariables(bindings));

So within this section of code you're getting your error; like Kal said it occurs when you're editing the ArrayList you're iterating over. To fix the issue change

for(Symbol s: args) {

To

for (int i = 0; i < args.size(); i++) {
    Symbol s = args.get(i);

answered Jul 7, 2011 at 17:49

Grambot

4,5356 gold badges30 silver badges44 bronze badges

1 Comment

Matthieu Over a year ago

WARNING for multi-thread use: you'll eventually get into some other kind of problems if you don't fix the root cause by properly synchronizing. E.g. args.get(i) will return null if ArrayList.clear() is called in between args.size() and args.get(). The code you provide works for single-thread only.

Collectives™ on Stack Overflow

ConcurrentModificationException on single-threaded code

2 Answers 2

1 Comment

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related