Why am I getting a java.util.ConcurrentModificationException in this example?

Question

In the following code sample, I don't understand why foo method throws ConcurrentModificationException. Please help!

private void foo() {
        synchronized (map) {
            if (map != null && !map.isEmpty()) {
                Set<String> it = map.keySet();
                ArrayList<String> delArray = new ArrayList<>();
                for (String key : it) {
                    MapItem mapItem = map.get(key);
                    if (mapItem != null) {
                        long wakeTime = System.currentTimeMillis() - mapItem.getTimestamp();
                        if (wakeTime > MY_THRESHOLD) {
                            if (mapItem.getLock() != null) {
                                mapItem.getLock().release();
                            }
                            delArray.add(key);
                        }
                    }
                }

                if (!delArray.isEmpty()) {
                    for (String key : delArray) {
                        map.remove(key);
                    }
                }
            }
        }
    }

I am getting exception on "for (String key : it) {" line

private static class MapItem {
        private PowerManager.WakeLock lock;
        private long timestamp;

        public MapItem(PowerManager.WakeLock lock, long timestamp) {
            this.lock = lock;
            this.timestamp = timestamp;
        }

        public PowerManager.WakeLock getLock() {
            return lock;
        }

        public long getTimestamp() {
            return timestamp;
        }
    }

where is the stacktrace? start from the line you get that exception at and then dive in further. — Naman, Sep 10 '17 at 07:57
Does `mapItem.getLock().release()` remove the item from the map by any chance? — Jon Skeet, Sep 10 '17 at 08:11
for each construct uses an iterator internally , kindly put a try catch and debug — Roshan, Sep 10 '17 at 08:15
post your MapItem Object and value for MY_THRESHOLD, I'll try debug — Drunken Daddy, Sep 10 '17 at 08:21
Do you have any other methods accessing your map without synchronization? — algrid, Sep 10 '17 at 08:42
in class constructor i do: map = Collections.synchronizedMap(new HashMap()); without synchronization. but my class is singleton — phnmnn, Sep 10 '17 at 08:50

score 1 · Answer 1 · answered Sep 10 '17 at 08:11

1

You are trying to change map while iterating over it, which is not allowed. ConcurrentModificationException:

If a single thread issues a sequence of method invocations that violates the contract of an object, the object may throw this exception. For example, if a thread modifies a collection directly while it is iterating over the collection with a fail-fast iterator, the iterator will throw this exception

answered Sep 10 '17 at 08:11

c0der

18,467
6
33
65

1

Where i'm trying to change map? – phnmnn Sep 10 '17 at 08:14
1

map.remove(key); out of loop – phnmnn Sep 10 '17 at 08:14
Try `Set it = new HashSet<>(map.keySet());` Do you still get the error ? – c0der Sep 10 '17 at 08:18
I can't simulate it. I'll try. – phnmnn Sep 10 '17 at 08:27

score 0 · Answer 2 · answered Sep 10 '17 at 10:23

I think you didn't put all your code here. Why the constructor if MapItem is WakeLockItem, and what
mapItem.getLock().release(); does? If I remove all PowerManager.WakeLock related code, I can execute it.

    private static class MapItem {
    private long timestamp;

    public MapItem(long timestamp) {
        this.timestamp = timestamp;
    }

    public long getTimestamp() {
        return timestamp;
    }
}

private Map<String, MapItem> map = new HashMap<>();

private void foo() {
    synchronized (map) {
        if (map != null && !map.isEmpty()) {
            Set<String> it = map.keySet();
            ArrayList<String> delArray = new ArrayList<>();
            for (String key : it) {
                MapItem mapItem = map.get(key);
                if (mapItem != null) {
                    long wakeTime = System.currentTimeMillis() - mapItem.getTimestamp();
                    if (wakeTime > 0) {
                        delArray.add(key);
                    }
                }
            }

            if (!delArray.isEmpty()) {
                for (String key : delArray) {
                    map.remove(key);
                }
            }
        }
    }
}

Why am I getting a java.util.ConcurrentModificationException in this example?

2 Answers2