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

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

core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/model/CElement.java

@@ -233,8 +233,7 @@ public abstract class CElement extends PlatformObject implements ICElement {
 	}
 	
 	public CElementInfo getElementInfo () throws CModelException {
-		CModelManager manager;
+		CModelManager manager = CModelManager.getDefault();
-		synchronized(manager = CModelManager.getDefault()){
 		Object info = manager.getInfo(this);
 		if (info == null) {
 			openHierarchy();
@@ -245,7 +244,6 @@ public abstract class CElement extends PlatformObject implements ICElement {
 		}
 		return (CElementInfo)info;
 	}
-	}
 
 	public String toString() {
 		return getElementName();

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);
+	}
+
 	/**
 	 * 
 	 */

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
 	 * <code>isOpen()</code>).
 	 */
 	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,7 +293,11 @@ 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
@@ -294,6 +305,10 @@ public abstract class Openable extends Parent implements IOpenable, IBufferChang
 		} catch (CModelException e) {
 			CModelManager.getDefault().removeInfo(this);
 			throw e;
+		} finally {
+			if (!hadTemporaryCache) {
+				manager.resetTemporaryCache();
+			}
 		}
 	}