concurrent modification on arraylist

Question

There are a lot of concurrent mod exception questions, but I'm unable to find an answer that has helped me resolve my issue. If you find an answer that does, please supply a link instead of just down voting.

So I originally got a concurrent mod error when attempting to search through an arraylist and remove elements. For a while, I had it resolved by creating a second arraylist, adding the discovered elements to it, then using removeAll() outside the for loop. This seemed to work, but as I used the for loop to import data from multiple files I started getting concurrent modification exceptions again, but intermittently for some reason. Any help would be greatly appreciated.

Here's the specific method having the problem (as well as the other methods it calls...):

public static void removeData(ServiceRequest r) {
    readData();
    ArrayList<ServiceRequest> targets = new ArrayList<ServiceRequest>();
    for (ServiceRequest s : serviceQueue) {  
    //ConcurrentModification Exception triggered on previous line
        if ( 
            s.getClient().getSms() == r.getClient().getSms() && 
            s.getTech().getName().equals(r.getTech().getName()) && 
            s.getDate().equals(r.getDate())) {
                JOptionPane.showMessageDialog(null, s.getClient().getSms() + "'s Service Request with " + s.getTech().getName() + " on " + s.getDate().toString() + " has been removed!");
                targets.add(s);
                System.out.print("targetted"); }
    }
    if (targets.isEmpty()) { System.out.print("*"); }
    else { 
        System.out.print("removed");
        serviceQueue.removeAll(targets); 
        writeData(); }
}
public static void addData(ServiceRequest r) {
    readData();
    removeData(r);
    if (r.getClient().getStatus().equals("MEMBER") || r.getClient().getStatus().equals("ALISTER")) {
        serviceQueue.add(r); } 
    else if (r.getClient().getStatus().equals("BANNED") || r.getClient().getStatus().equals("UNKNOWN")) {
        JOptionPane.showMessageDialog(null, "New Request failed: " + r.getClient().getSms() + " is " + r.getClient().getStatus() + "!", "ERROR: " + r.getClient().getSms(), JOptionPane.WARNING_MESSAGE);
    }
    else {
        int response = JOptionPane.showConfirmDialog(null, r.getClient().getSms() + " is " + r.getClient().getStatus() + "...", "Manually Overide?", JOptionPane.OK_CANCEL_OPTION);
        if (response == JOptionPane.OK_OPTION) {
            serviceQueue.add(r); }
    }
    writeData(); }

public static void readData() {
    try {
        Boolean complete = false;
        FileReader reader = new FileReader(f);
        ObjectInputStream in = xstream.createObjectInputStream(reader);
        serviceQueue.clear();
        while(complete != true) {   
            ServiceRequest test = (ServiceRequest)in.readObject();      
            if(test != null && test.getDate().isAfter(LocalDate.now().minusDays(180))) { 
                serviceQueue.add(test); }
                else { complete = true; }
        }
        in.close(); } 
    catch (IOException | ClassNotFoundException e) { e.printStackTrace(); } 
}
public static void writeData() {
    if(serviceQueue.isEmpty()) { serviceQueue.add(new ServiceRequest()); }
    try {
        FileWriter writer = new FileWriter(f);
        ObjectOutputStream out = xstream.createObjectOutputStream(writer);
        for(ServiceRequest r : serviceQueue) { out.writeObject(r); }
        out.writeObject(null);
        out.close(); }
    catch (IOException e) { e.printStackTrace(); }
}

EDIT

The changes cause the concurrent mod to trigger every time rather than intermittently, which I guess means the removal code is better but the error now triggers at it.remove();

public static void removeData(ServiceRequest r) {
    readData();
    for(Iterator<ServiceRequest> it = serviceQueue.iterator(); it.hasNext();) {
        ServiceRequest s = it.next();
        if ( 
            s.getClient().getSms() == r.getClient().getSms() && 
            s.getTech().getName().equals(r.getTech().getName()) && 
            s.getDate().equals(r.getDate())) {
                JOptionPane.showMessageDialog(null, s.getClient().getSms() + "'s Service Request with " + s.getTech().getName() + " on " + s.getDate().toString() + " has been removed!");
                it.remove();  //Triggers here (line 195)
                System.out.print("targetted"); }
    }
    writeData(); }

Exception in thread "AWT-EventQueue-0" java.util.ConcurrentModificatio nException at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:901) at java.util.ArrayList$Itr.next(ArrayList.java:851) at data.ServiceRequest.removeData(ServiceRequest.java:195) at data.ServiceRequest.addData(ServiceRequest.java:209) <...>

EDIT After some more searching, I've switch the for loop to:

        Iterator<ServiceRequest> it = serviceQueue.iterator();
        while(it.hasNext()) {

and it's back to intermittently triggering. By that I mean the first time I attempt to import data (the removeData method is being triggered from the addData method) it triggers the concurrent mod exception, but the next try it pushes past the failure and moves on to another file. I know there's a lot of these concurrent mod questions, but I'm not finding anything that helps in my situation so links to other answers are more than welcome...

"the for loop to import data from multiple files" can you post that? Also why is everything `static` and what is `serviceQueue`? — , Jun 12 '15 at 16:34
Stack trace, line number of exception, how many items, pattern in exception.. basically more details, please. — Praba, Jun 12 '15 at 16:35
I marked the line that the stack trace caught using commented text, and that also happens to be the for loop that is failing. Thanks for the quick response. @RC serviceQueue is an ArrayList of all the ServiceRequest's that have been generated, matched to an xml file. — Mercutio, Jun 12 '15 at 16:41
Can't you use Queue (like ArrayDeque) instead of List (like ArrrayList) ? — Nyamiou The Galeanthrope, Jun 12 '15 at 16:43
I'm not familiar with ArrayDeque. Searching now, do you have a recommended link @NyamiouTheGaleanthrope? — Mercutio, Jun 12 '15 at 16:44
The only link I have is https://docs.oracle.com/javase/7/docs/api/java/util/ArrayDeque.html. Basically, instead of add and remove you use push and pop and it may simplify what you are trying to achieve (I'm not sure what it is) and be faster. — Nyamiou The Galeanthrope, Jun 12 '15 at 18:21
Thanks I'll look into that. For further info I use these methods to match a list of ServiceRequests from a GUI to xml file. Normal usage appears fine, but when I'm importing old files (which I've had to do multiple times and seemed to work fine over the last month) I started getting these errors. It may have been slowly dropping out requests during normal usage but it's in these big imports that I can easily spot the issue. — Mercutio, Jun 12 '15 at 18:31

Nyamiou The Galeanthrope · Accepted Answer · 2015-06-12T18:27:00.123

1

This is not how to do it, to remove elements while going through a List you use an iterator. Like that :

List<ServiceRequest> targets = new ArrayList<ServiceRequest>();
for(Iterator<ServiceRequest> it = targets.iterator(); it.hasNext();) {
    ServiceRequest currentServReq = it.next();
    if(someCondition) {
        it.remove();
    }
}

And you will not get ConcurrentModificationException this way if you only have one thread.

If there is multiple threads involved in your code, you may still get ConcurrentModificationException. One way to solve this, is to use Collections.synchronizedCollection(...) on your collection (serviceQueue) and as a result you will get a synchronized collection that will not produce ConcurrentModificationException. But, you code may become very slow.

edited Jun 12 '15 at 18:27

answered Jun 12 '15 at 16:39

Nyamiou The Galeanthrope

1,178
9
23

So I replaced the if/else statement that removed the elements with an iterator (I'll put the new code up in my question) but I'm still getting the concurrent mod exception in the same spot (up in the for loop marked in the comments)... – Mercutio Jun 12 '15 at 17:00
Actually the last set of changes worked. I failed to overwrite the changes to the version I was running! Thanks again @NuamiouTheGaleanthrop – Mercutio Jun 12 '15 at 18:56

concurrent modification on arraylist

1 Answers1

Linked