Getting ConcurrentModificationException while modifying a HashMap in a thread class

Question

Hi I am running a thread service, the job of this thread is to check the age of a list items in a HashMap. When an item is older than say 5 seconds, I will have to delete the item from the HashMap. The below is the simplified code. But when the code attempts to delete the item from the HashMap, I get a java.util.ConcurrentModificationException.

I am populating the HashMap in the main() method in the original program. Can somebody please help me out with this ?
PS: The deleteFromTrackList() is being called by different clients across a network through RMI.

import java.util.*;

public class NotifierThread extends Thread {

    private HashMap<Integer, ArrayList> NotificationTrackList = new HashMap<Integer, ArrayList>();

    @Override
    public void run() {
        while (true) { // this process should run continuously
            checkNotifierList(getNotificationTrackList());
            try {
                Thread.sleep(1000);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }

    public HashMap<Integer, ArrayList> getNotificationTrackList() {
        return NotificationTrackList;
    }

    public void deleteFromTrackList(Integer messageID) {
        NotificationTrackList.remove(messageID);
    }

    public synchronized void checkNotifierList(HashMap list) {

        Set entries = list.entrySet();

        for (Iterator iterator = entries.iterator(); iterator.hasNext();) {
            Map.Entry<Integer, ArrayList> entry = (Map.Entry) iterator.next(); 

            ArrayList messageInfo = entry.getValue();
            Integer messageID = entry.getKey();

            messageInfo = new ArrayList((ArrayList) list.get(messageID));
            Long curTime = new Date().getTime();
            Long refTime = (Long) messageInfo.get(1);
            Long timeDiff = curTime - refTime;

            if (timeDiff > 5000) {
                // delete the entry if its older than 5 milliseconds and update
                // internal entry list
                deleteFromTrackList(messageID);
            }
        }
    }

    public static void main(String[] args) {
        new NotifierThread().start();
    }
}

This is the stacktrace I am getting at the console

Exception in thread "tracker" java.util.ConcurrentModificationException
    at java.util.HashMap$HashIterator.nextEntry(Unknown Source)
    at java.util.HashMap$EntryIterator.next(Unknown Source)
    at java.util.HashMap$EntryIterator.next(Unknown Source)
    at NotifierThread.checkNotifierList(NotifierThread.java:32)
    at NotifierThread.run(NotifierThread.java:10)

JB Nizet · Accepted Answer · 2011-10-03 16:31:23Z

4

The only way to remove an entry from a map while iterating over it is to remove it using the iterator. Use

iterator.remove();

instead of

deleteFromTrackList(messageID);

Note that the same applies to all the collections (List, Set, etc.)

Also, note that your design is not thread-safe, because you let other threads access the map in an unsynchronized way.

answered Oct 3, 2011 at 16:31

JB Nizet

694k94 gold badges1.3k silver badges1.3k bronze badges

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

2 Comments

Rakesh Over a year ago

I tried iterator.remove(); But I need to remove the item outside of the loop, as the deleteFromTrackList(messageID); is being called by different clients across network through RMI.

JB Nizet Over a year ago

The fact that you need to have a separate method to enable other clients to remove a message ID doesn't prevent you to use iterator.remove when you remove it while iterating. You should just make sure that no other thread calls deleteFromTrackList while you're iterating over the list. Don't let the map escape your class, and synchronize every access to the map.

tcb · Accepted Answer · 2011-10-03 16:32:48Z

0

Your code isn't complectly synchronized. Try to change

public void deleteFromTrackList(Integer messageID) {

to

public synchronized void deleteFromTrackList(Integer messageID) {

answered Oct 3, 2011 at 16:32

tcb

2,83222 silver badges20 bronze badges

2 Comments

John B Over a year ago

This is probably a good thing to fix, but it would not fix the ConcurrentModificationException

JB Nizet Over a year ago

And the method getNotificationTrackList() would also have to be removed (or modified to be synchronized and return a copy of the map) to make it thread-safe

John B · Accepted Answer · 2011-10-03 18:09:31Z

0

Correct. You cannot modify a Map while iterating over it without using the iterator directly. There are a couple main options.

Create a List of elements that should be removed. Add each expired element to the List in the loop. After the loop, remove the elements in the List from the Map.
Use Guava's filter capability. Maps.filterEntries This creates a new Map however which may not work for what you are trying to do.

Since you have a multi-threaded system. You may want to consider immutability as your friend. Rather than blocking threads over your entire check for stale loop, you could use an ImmutableMap which would be more thread-safe with better performance.

edited Oct 3, 2011 at 18:09

answered Oct 3, 2011 at 16:31

John B

33.1k8 gold badges80 silver badges100 bronze badges

2 Comments

Rakesh Over a year ago

I have to modify the list before completion of the loop. Is there any way to do this ? Meanwhile I will check the Guava's filter capability. Thanks.

John B Over a year ago

Using the Iterator.remove method. Are you sure that you need to remove from within the loop. Depending on your synchronization, no other thread will have access to the map which you are checking it so I would think I would not matter.

Rakesh · Accepted Answer · 2011-10-06 14:47:51Z

0

Thanks for your answers guys... I have found the solution for my question, instead of using HashMap, I am using ConcurrentHashMap. This solved my problem. Thanks again !

answered Oct 6, 2011 at 14:47

Rakesh

4,3424 gold badges35 silver badges58 bronze badges

Comments

99Sono · Accepted Answer · 2012-07-03 10:44:24Z

0

In reality you do not even need concurrent access to an hash map to get a concurrency exception. In fact, a single thread is quite enough.

For example, You may create a loop based on the hash map map.keySet().iterator(), And, while you are within this loop, your (single) thread decides to remove an element from the hash map. (Not a good idea while the iterator is open.) In the next request to the iterator().next() you will get your concurrency exception.

So careful with that.

answered Jul 3, 2012 at 10:44

99Sono

3,70734 silver badges42 bronze badges

Collectives™ on Stack Overflow

Getting ConcurrentModificationException while modifying a HashMap in a thread class

5 Answers 5

2 Comments

2 Comments

2 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

2 Comments

2 Comments

2 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related