CODEWITHSUNDEEP
javamultithreadingservletsconcurrency
perissf
perissf

Reputation: 16273

List ConcurrentModificationException in servlet

It's plenty of questions regarding ConcurrentModificationException for ArrayList objects, but I could not find yet an answer to my problem.

In my servlet I have an ArrayList as a member object:

List myList<Object> = new ArrayList<Object> (...);

The list must be shared among users and sessions.

In one of the servlet's methods, method1, I need to iterate over the ArrayList items, and eventually add clear the list after the iteration. Here a snippet:

for (Object o : myList) {
    // read item o
}
myList.clear();

In another method, method2, I simply add a new Item to the list.

Most of the times the method ends its job without errors. Sometimes, probably due to the concurrent invocation of this method by different users, I get the famous java util.ConcurrentModificationException exception. Should I define my List as:

List myList = Collections.synchronizedList(new ArrayList(...));

Would this be enough or am I missing something? What's behind the scenes? When there is a possible concurrency, is the second thread held in standby by the container?

EDIT: I have added the answers to some comments.

Upvotes: 1

Views: 565

Answers (5)

John Vint
John Vint

Reputation: 40256

This is kind of an add onto Peter Lawery's answer. But since copying wouldn't effect you too negatively you can do a mixture of copying with synchronization.

    private final List<Object> myList = new ArrayList<Object>();
    public void iterateAndClear(){
      List<Object> local = null; 
      synchronized(myList){
          local  = new ArrayList<Object>(myList);
          myList.clear();
      }
      for(Object o  : local){
         //read o
      }
    }
    public void add(Object o){
      synchronized(myList){ 
         myList.add(o);
      }
    }

Here you can iterate over o elements without fear of comodifications (and outside of any type of synchronization), all while myList is safely cleared and added to.

Upvotes: 1

Vilas
Vilas

Reputation: 1455

Using a synchronized list will not solve your problem. The core of the problem is that you are iterating over a list and modifying it at the same time. You need to use mutual exclusion mechanisms (synchronized blocks, locks etc) to ensure that they do not happen at the same time. To elaborate, if you start with:

methodA() {
  iterate over list {
  }
  edit list;
}

methodB() {
  edit list;
}

If you use a synchronized list, what you essentially get is:

methodA() {
  iterate over list {
  }
  synchronized {
    edit list;
  }
}

methodB() {
  synchronized {
    edit list;
  }
}

but what you actually want is:

methodA() {
  synchronized {
    iterate over list {
    }
    edit list;
  }
}

methodB() {
  synchronized {
    edit list;
  }
}

Upvotes: 4

AntonyM
AntonyM

Reputation: 1604

You need to lock not just over the method accesses but over your use of the list.

So if you allocate a paired Object like:

Object myList_LOCK = new Object();

then you can lock that object whenever you are accessing the List, like this:

synchronized(myList_LOCK) {

//Iterate through list AND modify all within the same lock

}

at the moment the only locking you're doing is within the individual methods of the List, which isn't enough in your case because you need atomicity over the entire sequence of iteration and modification.

You could use the actual object (myList) to lock rather than a paired object but in my experience you are better off using another dedicated object as it avoids unexpected deadlock conditions that can arise as a result of the code internal to the object locking on the object itself.

Upvotes: 1

Peter Lawrey
Peter Lawrey

Reputation: 533670

Just using synchronizedList makes all methods thread safe EXCEPT Iterators.

I would use CopyOnWriteArrayList. It is thread safe and doesn't produce ConcurrentModificationException.

Upvotes: 2

jpredham
jpredham

Reputation: 2199

ConcurrentModificaitonException occurs when you attempt to modify a collection while you're iterating through it. I imagine that the error only gets thrown when you perform some conditional operation.

I'd suggest pushing the values you want to add/remove into a separate list and performing the add /remove after you're done iterating.

Upvotes: 1

Related Questions