CODEWITHSUNDEEP
javaconcurrencysynchronizedconcurrentmodificationsynchronized-block
David
David

Reputation: 4817

ConcurrencyException

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();
            }
        }
    }
}

Upvotes: 0

Views: 1026

Answers (2)

Peter Lawrey
Peter Lawrey

Reputation: 533462

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();
        }
    }
}

Upvotes: 2

kosa
kosa

Reputation: 66637

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

Upvotes: 0

Related Questions