ConcurrencyException

Question

private static HashMap<String, FileInfo> sFileInfoObjectList = new CacheLinkedHashMap<String, FileInfo>();

public static synchronized FileInfo getFileInfoForProvider(...) {
 FileInfo foundFileInfo = null;

 (...)

 foundFileInfo = sFileInfoObjectList.get(hashEntryKey);

 (...)

 sFileInfoObjectList.put(hashEntryKey, foundFileInfo);

 (...)
}

public static synchronized void removeFileInfoForProvider(final int providerId) {
    Thread thread = new Thread() {
        @Override
        public void run() {
            Iterator<Entry<String, FileInfo>> it = sFileInfoObjectList.entrySet().iterator();
            while (it.hasNext()) {
                Entry<String, FileInfo> pair = it.next();
                FileInfo info = pair.getValue();
                if (providerId == info.mProvider) {                            
                    it.remove();
                }
            }
        }
    };
}

I am getting a ConccurentModificationException in the run() method. I tried following and it did not work:

public void run() {
    synchronized(sFileInfoObjectList) {
        Iterator<Entry<String, FileInfo>> it = sFileInfoObjectList.entrySet().iterator();
        while (it.hasNext()) {
            Entry<String, FileInfo> pair = it.next();
            FileInfo info = pair.getValue();
            if (providerId == info.mProvider) {                            
                it.remove();
            }
        }
    }
}

Peter Lawrey · Accepted Answer · 2013-01-02T23:16:23.540

2

The run() is not in a synchronized block because it is run on another thread. You could use synchronized in your run method but it would be much simpler to use a concurrent collection which does throw this error. E.g. ConcurrentHashMap.

BTW: I wouldn't start a thread each time as it could be more expensive than just iterating over the collection. I would use a thread pool, or do it in the current thread.

Replace your method with this

public static void removeFileInfoForProvider(final int providerId) {
    Thread thread = new Thread() {
        @Override
        public void run() {
            removeFileInfoForProvider0(providerId);
        }
    };
    thread.start();
}

static synchronized void removeFileInfoForProvider0(final int providerId) {
    Iterator<Entry<String, FileInfo>> it = sFileInfoObjectList.entrySet().iterator();
    while (it.hasNext()) {
        Entry<String, FileInfo> pair = it.next();
        FileInfo info = pair.getValue();
        if (providerId == info.mProvider) {                            
            it.remove();
        }
    }
}

edited Jan 02 '13 at 23:16

answered Jan 02 '13 at 15:55

Peter Lawrey

525,659
79
751
1,130

but I am using a LinkedHashMap and the method is synchronized – David Jan 02 '13 at 15:56
The run() method is run in a different thread and it is not synchronized. The method you have synchronized doesn't touch the collection so it doesn't need to be synchronized. – Peter Lawrey Jan 02 '13 at 15:57
You have to lock on the same object which is the class in your case (as the other locks are static) A simpler approach is to have it call a synchronized method on the class. BTW since your collection is single threaded starting another thread won't help very much. – Peter Lawrey Jan 02 '13 at 15:59
I do not know, but all your comments are not self explaining and do not help me – David Jan 02 '13 at 20:35
Since you don't appear to understand any of the instructions given, perhaps you can just replace your code with the above which will work. – Peter Lawrey Jan 02 '13 at 23:17
Thanks Peter, it worked. I wrote a test to reproduce the Exception and with your changes the exception does not occur anymore. Anyway I must still analyse carefully why and read more about it to understand it better, but thanks anyways. – David Jan 03 '13 at 09:12
@David You have to think about which object is being locked for which thread. When you use static synchronized you are locking the class object e.g. `MyClass.class` for the current thread. Using a different object or starting another thread is not the same. – Peter Lawrey Jan 03 '13 at 09:20

score 0 · Answer 2 · answered Jan 02 '13 at 15:53

0

You haven't synchronized remove() call for the Map. If multiple threads try to call run() same time, it will throw ConcurrentModificationException which is the reason in your case.

You may need to use either synchronized on run() (or) use concurrent collection like ConcurrentHashMap

answered Jan 02 '13 at 15:53

kosa

65,990
13
130
167

I assume the OP believes making `removeFileInfoForProvider` synchronized is enough. – Peter Lawrey Jan 02 '13 at 15:56
If i synchronize over the collection variable, I do not need to synchronize remove() – David Jan 02 '13 at 20:37
"If i synchronize over the collection variable" do you mean, using Concurrent collection? – kosa Jan 02 '13 at 20:39
no, see my first post after I wrote: I tried following and it did not work: – David Jan 02 '13 at 20:45
Approach should work fine, but issue could be that you are synchronizing on the object which is being modified, which may not work. Use some other to maintain synch lock. – kosa Jan 02 '13 at 20:49

ConcurrencyException

2 Answers2