From 398307d9bb1f9caaa0bc825ecc83459cc0a1dd83 Mon Sep 17 00:00:00 2001 From: Andrey Loskutov Date: Mon, 18 Sep 2017 17:01:06 +0200 Subject: [PATCH] Bug 522411 - ConcurrentModificationException below CModelListener.addLastRecentlyUsed (thrown in LinkedHashMap$LinkedHashIterator.nextNode) CModelListener reacts on all the resource events, but is NOT MT-safe due the not guarded access to the "fLRUs" map, which can be iterated AND modified at same time by different threads. This commit introduces synchronization on the "fLRUs" map field. This is safe as the field is private and not exposed to other objects, and the code inside synchronized block does not call into other locks which might interfere. Change-Id: I3e601f02e93e40a1454c9a581fa46378904eb3dc Signed-off-by: Andrey Loskutov --- .../internal/core/pdom/CModelListener.java | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/CModelListener.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/CModelListener.java index f6ef7442884..bfc8231223d 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/CModelListener.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/CModelListener.java @@ -39,7 +39,7 @@ public class CModelListener implements IElementChangedListener, IResourceChangeL public static boolean sSuppressUpdateOfLastRecentlyUsed = false; private PDOMManager fManager; - private LinkedHashMap fLRUs= new LinkedHashMap(UPDATE_LR_CHANGED_FILES_COUNT, 0.75f, true) { + private final LinkedHashMap fLRUs= new LinkedHashMap(UPDATE_LR_CHANGED_FILES_COUNT, 0.75f, true) { @Override protected boolean removeEldestEntry(Map.Entry eldest) { return size() > UPDATE_LR_CHANGED_FILES_COUNT; @@ -131,23 +131,25 @@ public class CModelListener implements IElementChangedListener, IResourceChangeL } if (count > 0) { - if (addLRUs) { - for (final ITranslationUnit tu : fLRUs.keySet()) { - if (tu.getResource().exists()) { - final ICProject cproject= tu.getCProject(); - DeltaAnalyzer analyzer= changeMap.get(cproject); - if (analyzer == null) { - analyzer= new DeltaAnalyzer(); - changeMap.put(cproject, analyzer); + synchronized(fLRUs) { + if (addLRUs) { + for (final ITranslationUnit tu : fLRUs.keySet()) { + if (tu.getResource().exists()) { + final ICProject cproject= tu.getCProject(); + DeltaAnalyzer analyzer= changeMap.get(cproject); + if (analyzer == null) { + analyzer= new DeltaAnalyzer(); + changeMap.put(cproject, analyzer); + } + analyzer.getForcedList().add(tu); } - analyzer.getForcedList().add(tu); } } - } - count= Math.min(count, newLRUs.length); - for (int i = 0; i < count; i++) { - final ITranslationUnit tu = newLRUs[i]; - fLRUs.put(tu, tu); + count= Math.min(count, newLRUs.length); + for (int i = 0; i < count; i++) { + final ITranslationUnit tu = newLRUs[i]; + fLRUs.put(tu, tu); + } } } }