Mattia
Mattia

Reputation: 140

java.util.ConcurrentModificationException in a Servlet

EDIT: I understand my mistake. I don't why but I was thinking that after the previousItems.add(p); is executed it was going out from the for loop. I've solved by adding a break

Other questions about this Exception didn't help me to get a solution.

I have a Servlet that is called when I add an item in the cart from another page.

I have an ArrayList<Product> and I iterate through the List to check if the same Product that I'm trying to add in already in the list. If it's already there I update its quantity otherwise I add the new Product in the List.

Everything is fine if add always the same Product, the Exception occurs when I add a different Product. So I think the problem in the code is after the else (commented with "This"), because it's executed if the Product is different.

@WebServlet(name = "AddCart", urlPatterns = {"/AddCart"})
public class AddCart extends HttpServlet {

    @Override
    protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
        HttpSession session = request.getSession();
        ArrayList<Product> previousItems = (ArrayList<Product>) session.getAttribute("previousItems");
        Product p = (Product) session.getAttribute("currentProduct");
        if (previousItems == null) {
            previousItems = new ArrayList<Product>();
        }

            if (p != null) {
                if (previousItems.size()>0) {
                    for (Product p1 : previousItems) {
                        if (p1.getId() == p.getId()) {
                            p1.addQuantity();
                        } else { //This
                            previousItems.add(p);
                        }
                    }
                } else {
                    previousItems.add(p);
                }
            }

        session.setAttribute("previousItems", previousItems);
        response.sendRedirect("cart.jsp");
    }
}

I've also tried to remove the synchronized same Exception.

And this is the HTTP Status 500 – Internal Server Error

java.util.ConcurrentModificationException

java.util.ArrayList$Itr.checkForComodification(ArrayList.java:901) java.util.ArrayList$Itr.next(ArrayList.java:851) servlets.AddCart.doGet(AddCart.java:36) javax.servlet.http.HttpServlet.service(HttpServlet.java:635) javax.servlet.http.HttpServlet.service(HttpServlet.java:742) org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52)

Upvotes: 1

Views: 4491

Answers (4)

Imran
Imran

Reputation: 1902

Use, CopyOnWriteArrayList instead of ArrayList if you just want to avoid ConcurrentModificationException.

Upvotes: 0

You cannot update a list that you are iterating, this cause ConcurrentModificationException

for (Product p1 : previousItems) {
    previousItems.add(p); //you cant update the previousItems list here
}

Instead what you can do is:

ArrayList<Product> auxList = new ArrayList<>();
for (Product p1 : previousItems) {
    //...
    //else...
        auxList.add(p);
}

and after:

previousItems.addAll(auxList);

Upvotes: 1

Andy Turner
Andy Turner

Reputation: 140484

You can't add an item to a list you are iterating with an enhanced for loop. This is because you are modifying the internal state of the list; whilst it is possible to handle this, most iterator implementations don't handle state changes of the underlying collection, in order to stay simple for the vast, vast majority of use cases.

Instead of this:

for (Product p1 : previousItems) {
    previousItems.add(p); // Simplified
}

If you want p to be in the list after, put it into another list, then add that list after iteration:

List<Product> other = new ArrayList<>();
for (Product p1 : previousItems) {
    other.add(p);
}
previousItems.addAll(other);

Upvotes: 6

Aleksei Budiak
Aleksei Budiak

Reputation: 921

This exception is expected when you modify ArrayList inside of for loop. Internally, for loop uses Iterator and modification of non thread safe collections during iteration is not allowed. You can read this article to find out more.

But, I'd recommend to change your logic the following way:

if (p != null) {
  // Check if previousItems already contains the product
  if (!previousItems.contains(p)) {
    // If it doesn't, add the product
    previousItems.add(p);
  }
}

Upvotes: 1

Related Questions