Iterators and ConcurrentModificationException

Question

I have a LinkedList<JLabel> and using it on a thread to add or remove them from a JPanel, the problem is that sometimes the exception happens and the application crashes. In some posts they talk about synchronize my list, others say that I should have an iterator, so I'm a bit confused.

I will add some code for make it easy to understand:

public class MyPanel extends JPanel {

(...)
private LinkedList<JLabel> labels;
(...)

public MyPanel(Color corLabel, Color back, Font text) {
(...)
labels = new LinkedList<>();
(...)
}

this is where the Linked List get initialized now i have 1 method and 1 thread using it:

 public void addMensagem(MensagemParaEcra msg) {

    JLabel lbl = new JLabel();
    lbl.setText(("- " + msg.getTexto() + " -"));
    lbl.setForeground(color);
    lbl.setFont(textFont);
    lbl.setOpaque(true);
    lbl.setBackground(backg);

    labels.add(lbl);
    MyPanel.this.add(lbl);

}

this one just configure the label and add it to the panel and to the linked list

private void start() {
    final Runnable running = new Runnable() {
        @Override
        public void run() {
            MyPanel.this.repaint();

        }
    };

    Thread t = new Thread() {
        public void run() {
            while (true) {
                // for each label
                for (JLabel lb : labels) {
                    if (lb.getLocation().x + lb.getWidth() < 0) {
                        if (msgsRemover.isEmpty() == true) {
                            //remove the label if she is out of the monitor
                            MyPanel.this.remove(lb);
                            // and add the label
                            MyPanel.this.add(lb);

                            MyPanel.this.repaint();
                            MyPanel.this.validate();
                        } else {
                            // if there is some message to remove
                            for (String s : msgsRemover) {
                                if (lb.getText().toString().equals(s)) {
                                    // remove the visual label   
                                    MyPanel.this.remove(lb);
                                    MyPanel.this.repaint();
                                    // remove the message that need to be removed from the linked list
                                    msgsRemover.remove(s);
                                    // remove the label from the JLabel list
                                    labels.remove(lb);

                                } else {
                                    // if there is no message to be removed, they will just continue
                                    // going to the end of the queue
                                    MyPanel.this.remove(lb);
                                    MyPanel.this.add(lb);

                                    MyPanel.this.repaint();
                                    MyPanel.this.validate();
                                }
                            }

                        }

                    }
                    lb.setLocation(lb.getLocation().x - 3, 0);

                }
                repaint();
                try {
                    SwingUtilities.invokeAndWait(running);
                    sleep(30);
                } catch (InterruptedException ex) {
                    Logger.getLogger(MyPanel.class.getName()).log(Level.SEVERE, null, ex);
                } catch (InvocationTargetException ex) {
                    Logger.getLogger(MyPanel.class.getName()).log(Level.SEVERE, null, ex);
                }

            }
        }
    };
    t.start();
}

This is the code where I got the error, sometimes the program crashes and for what it says the problem is on:

for (JLabel lb : labels)

The stacktrace:

Exception in thread "Thread-2" java.util.ConcurrentModificationException
at java.util.LinkedList$ListItr.checkForComodification(LinkedList.java:953)
at java.util.LinkedList$ListItr.next(LinkedList.java:886)
at smstest.MyPanel$3.run(MyPanel.java:110)

So, i used an iterator and it's working, but now i face another problem... there is the code:

for(Iterator<String> it2 = msgsRemover.iterator(); it.hasNext();){
                                String s = it2.next();

                                if (lb.getText().toString().equals(s)) {
                                    // remove the visual label   
                                    MyPanel.this.remove(lb);
                                    MyPanel.this.repaint();
                                    // remove the message that need to be removed from the linked list
                                    it2.remove();
                                    // remove the label from the JLabel list
                                    it.remove();

so, now my problem is in String s = it2.next(); The stacktrace is:

Exception in thread "Thread-2" java.util.NoSuchElementException
at java.util.LinkedList$ListItr.next(LinkedList.java:888)
at smstest.MyPanel$3.run(MyPanel.java:131)

Any clue how can i solve it? Thank you in advance

You can't add or remove from a `List` directly while iterating over it. Which you do here: `labels.remove(lb);` — Sotirios Delimanolis, Sep 21 '13 at 16:05
You must use an iterator to remove an item from the collection you're navigating. By the way, it will be better if you post the stacktrace where it shows the problem description. — Luiggi Mendoza, Sep 21 '13 at 16:06
Also, this problem will persist if two or more threads modify the `LinkedList` at the same time. The solution for this would be using a data structure that supports concurrency modification as [`ArrayBlockingQueue`](http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/ArrayBlockingQueue.html). — Luiggi Mendoza, Sep 21 '13 at 16:10

score 3 · Accepted Answer · answered Sep 21 '13 at 16:20

3

You have to use an Iterator to remove from a List while iterating:

final Iterator<JLabel> labelIter = labels.iterator();
while(labelIter.hasNext()) {
    final JLabel label = labelIter.next();
    //do stuff with label
    labelIter.remove();
}

More importantly, you cannot do what you are trying to do. Swing is not thread safe.

You cannot make changes to swing components off of the EDT.

Please read this before you proceed. Otherwise you will end up experiencing race hazards and seemingly random GUI problems.

answered Sep 21 '13 at 16:20

Boris the Spider

59,842
6
106
166

And `ListIterator` lets you add as well. – Jeremy Sep 21 '13 at 16:21
Doesn't work... i have a scrolling text, that's what the labels are for, if i do that, the text will not roll.. – João Silva Sep 21 '13 at 16:25
I manage to make it work with the iterator but now i got another problem, i will edit the post – João Silva Sep 21 '13 at 17:46
You seem to have confused your variables. You have `it` and `it2`. Go through your code **carefully** and make sure that you are using the right one at the right time. I'll give you a clue, you're not. – Boris the Spider Sep 21 '13 at 18:15
P.S. you are going to have to completely rewrite your code to comply with Swing's threading policy... – Boris the Spider Sep 21 '13 at 18:20
I notice that about it and it2... yeah, i notice that, but i guess that for now it will stay like this, i added a button for recreate the frame when it crash, so for a beta version it should be ok... and i dont have complete my degree in computer engineering yet, so i'm not a expert programmer... anyway, thank you a lot for the help :) – João Silva Sep 21 '13 at 21:05

score 0 · Answer 2 · answered Sep 21 '13 at 16:07

0

Iterators throww ConcurrentModificationException if data structure is modified (you add or remove stuff) while iterating.

To fix the issue you can clone the labels structure and iterate on the clone.

answered Sep 21 '13 at 16:07

Claudiu

1,469
13
21

I tryed the clone, but didnt work :\ – João Silva Sep 21 '13 at 16:19
@João how did you create the clone? Have you tried creating a `local` (not a class member) `Arraylist a` for instance and do `a.putAll(labels)` inside and iterate over that `a`? – Claudiu Sep 21 '13 at 16:23
I tried in 2 ways... ---- LinkedList clonned = (LinkedList) labels.clone(); ------ LinkedList clonned2 = new LinkedList<>(); ----- clonned2.addAll(labels); – João Silva Sep 21 '13 at 16:31
You should not get that exception with another list that you are not altering while iterating. – Claudiu Sep 21 '13 at 16:45
with this i dont get the exception, the problem is that the scrolling text doesnt scroll... stay fixed... – João Silva Sep 21 '13 at 16:51

score 0 · Answer 3 · answered Sep 21 '13 at 16:25

0

You cannot use LinkedList in multi-threaded context use java.util.concurrent.CopyOnWriteArryayList instead

answered Sep 21 '13 at 16:25

Evgeniy Dorofeev

133,369
30
199
275

Iterators and ConcurrentModificationException

3 Answers3