diff --git a/core/org.eclipse.cdt.core/ChangeLog b/core/org.eclipse.cdt.core/ChangeLog index d842d45e2db..2784d08d783 100644 --- a/core/org.eclipse.cdt.core/ChangeLog +++ b/core/org.eclipse.cdt.core/ChangeLog @@ -1,3 +1,13 @@ +2004-06-04 Alain Magloire + + Potential deadlock in CElement.getElementInfo() + We can no longer synch on CModelMager. We need + to do some fine grained lock for the LRU caching. + + * model/org/eclipse/cdt/internal/core/model/CElement.java + * model/org/eclipse/cdt/internal/core/model/CModelManager.java + * model/org/eclipse/cdt/internal/core/model/Openable.java + 2004-06-03 Alain Magloire The Elf class should not depend of GNU diff --git a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/CElement.java b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/CElement.java index 9fcef63b005..556f9ecbb86 100644 --- a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/CElement.java +++ b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/CElement.java @@ -233,18 +233,16 @@ public abstract class CElement extends PlatformObject implements ICElement { } public CElementInfo getElementInfo () throws CModelException { - CModelManager manager; - synchronized(manager = CModelManager.getDefault()){ - Object info = manager.getInfo(this); + CModelManager manager = CModelManager.getDefault(); + Object info = manager.getInfo(this); + if (info == null) { + openHierarchy(); + info= manager.getInfo(this); if (info == null) { - openHierarchy(); - info= manager.getInfo(this); - if (info == null) { - throw newNotPresentException(); - } + throw newNotPresentException(); } - return (CElementInfo)info; } + return (CElementInfo)info; } public String toString() { diff --git a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/CModelManager.java b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/CModelManager.java index 86b58a912f1..41b2d4af180 100644 --- a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/CModelManager.java +++ b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/CModelManager.java @@ -109,6 +109,11 @@ public class CModelManager implements IResourceChangeListener, ICDescriptorListe */ protected Map elementsOutOfSynchWithBuffers = new HashMap(11); + /* + * Temporary cache of newly opened elements + */ + private ThreadLocal temporaryCache = new ThreadLocal(); + /** * Infos cache. */ @@ -986,31 +991,73 @@ public class CModelManager implements IResourceChangeListener, ICDescriptorListe /** * Returns the info for the element. */ - public Object getInfo(ICElement element) { + public synchronized Object getInfo(ICElement element) { + HashMap tempCache = (HashMap)this.temporaryCache.get(); + if (tempCache != null) { + Object result = tempCache.get(element); + if (result != null) { + return result; + } + } return this.cache.getInfo(element); } + /** * Returns the info for this element without * disturbing the cache ordering. */ - protected Object peekAtInfo(ICElement element) { + protected synchronized Object peekAtInfo(ICElement element) { + HashMap tempCache = (HashMap)this.temporaryCache.get(); + if (tempCache != null) { + Object result = tempCache.get(element); + if (result != null) { + return result; + } + } return this.cache.peekAtInfo(element); } /** * Puts the info for a C Model Element */ - protected void putInfo(ICElement element, Object info) { + protected synchronized void putInfo(ICElement element, Object info) { this.cache.putInfo(element, info); } /** * Removes the info of this model element. */ - protected void removeInfo(ICElement element) { + protected synchronized void removeInfo(ICElement element) { this.cache.removeInfo(element); } + /* + * Returns the temporary cache for newly opened elements for the current thread. + * Creates it if not already created. + */ + public HashMap getTemporaryCache() { + HashMap result = (HashMap)this.temporaryCache.get(); + if (result == null) { + result = new HashMap(); + this.temporaryCache.set(result); + } + return result; + } + + /* + * Returns whether there is a temporary cache for the current thread. + */ + public boolean hasTemporaryCache() { + return this.temporaryCache.get() != null; + } + + /* + * Resets the temporary cache for newly created elements to null. + */ + public void resetTemporaryCache() { + this.temporaryCache.set(null); + } + /** * */ diff --git a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/Openable.java b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/Openable.java index f60a1964ad5..48d5f9fda26 100644 --- a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/Openable.java +++ b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/Openable.java @@ -66,13 +66,12 @@ public abstract class Openable extends Parent implements IOpenable, IBufferChang * removing the current infos, generating new infos, and then placing * the new infos into the C Model cache tables. */ - protected void buildStructure(OpenableInfo info, IProgressMonitor monitor) throws CModelException { + protected void buildStructure(OpenableInfo info, HashMap newElements, IProgressMonitor monitor) throws CModelException { if (monitor != null && monitor.isCanceled()) return; // remove existing (old) infos removeInfo(); - HashMap newElements = new HashMap(11); info.setIsStructureKnown(generateInfos(info, monitor, newElements, getResource())); CModelManager.getDefault().getElementsOutOfSynchWithBuffers().remove(this); for (Iterator iter = newElements.keySet().iterator(); iter.hasNext();) { @@ -122,9 +121,6 @@ public abstract class Openable extends Parent implements IOpenable, IBufferChang * the structure of this element. */ protected abstract boolean generateInfos(OpenableInfo info, IProgressMonitor pm, Map newElements, IResource underlyingResource) throws CModelException; - //protected boolean generateInfos(OpenableInfo info, IProgressMonitor pm, Map newElements, IResource underlyingResource) throws CModelException { - // return false; - //} /** * @see org.eclipse.cdt.core.model.IOpenable#getBuffer() @@ -231,7 +227,16 @@ public abstract class Openable extends Parent implements IOpenable, IBufferChang public void makeConsistent(IProgressMonitor pm, boolean forced) throws CModelException { if (!isConsistent() || forced) { - buildStructure((OpenableInfo)getElementInfo(), pm); + CModelManager manager = CModelManager.getDefault(); + boolean hadTemporaryCache = manager.hasTemporaryCache(); + try { + HashMap newElements = manager.getTemporaryCache(); + buildStructure((OpenableInfo)getElementInfo(), newElements, pm); + } finally { + if (!hadTemporaryCache) { + manager.resetTemporaryCache(); + } + } } } @@ -273,8 +278,10 @@ public abstract class Openable extends Parent implements IOpenable, IBufferChang * isOpen()). */ protected void openWhenClosed(IProgressMonitor pm) throws CModelException { + CModelManager manager = CModelManager.getDefault(); + boolean hadTemporaryCache = manager.hasTemporaryCache(); try { - + HashMap newElements = manager.getTemporaryCache(); // 1) Parent must be open - open the parent if necessary openParent(pm); @@ -286,14 +293,22 @@ public abstract class Openable extends Parent implements IOpenable, IBufferChang } // 3) build the structure of the openable - buildStructure(info, pm); - + buildStructure(info, newElements, pm); + + //if (!hadTemporaryCache) { + // manager.putInfos(this, newElements); + //} + // if any problems occuring openning the element, ensure that it's info // does not remain in the cache (some elements, pre-cache their info // as they are being opened). } catch (CModelException e) { CModelManager.getDefault().removeInfo(this); throw e; + } finally { + if (!hadTemporaryCache) { + manager.resetTemporaryCache(); + } } }