Fixed a deadlock.

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

@@ -11,14 +11,13 @@
  ******************************************************************************/
 package org.eclipse.cdt.internal.core.model;
 
-import java.util.concurrent.Semaphore;
-
 import org.eclipse.cdt.core.CCorePlugin;
 import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
 import org.eclipse.cdt.core.index.IIndex;
 import org.eclipse.cdt.core.index.IIndexManager;
 import org.eclipse.cdt.core.model.ILanguage;
 import org.eclipse.cdt.core.model.ITranslationUnit;
+import org.eclipse.cdt.internal.core.dom.parser.ASTTranslationUnit;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.ISafeRunnable;
@@ -74,11 +73,6 @@ public class ASTCache {
 	private ITranslationUnit fActiveTU;
 	/** The cached AST if any */
 	private IASTTranslationUnit fAST;
-	/**
-	 * The semaphore controlling exclusive access to the cached AST.
-	 * <code>null</code> when fAST is <code>null</code>.
-	 */
-	private Semaphore fASTSemaphore;
 	/** 
 	 * The timestamp of the last index write access at the time
 	 * the AST got cached. A cached AST becomes invalid on any index
@@ -96,12 +90,11 @@ public class ASTCache {
 	}
 
 	/**
-	 * Returns a shared translation unit AST for the given
+	 * Returns a shared translation unit AST for the given translation unit.
-	 * translation unit.
 	 * <p>
-	 * Clients are not allowed to modify the AST and must
+	 * Clients are not allowed to modify the AST and must hold an index read
-	 * hold a read lock prior to calling this method and continue
+	 * lock prior to calling this method and continue to hold the lock as long
-	 * to hold the lock as long as the AST is being used.
+	 * as the AST is being used.
 	 * </p>
 	 *
 	 * @param tUnit				the translation unit
@@ -110,7 +103,8 @@ public class ASTCache {
 	 * @param progressMonitor	the progress monitor or <code>null</code>
 	 * @return					the AST or <code>null</code> if the AST is not available
 	 */
-	private IASTTranslationUnit getAST(ITranslationUnit tUnit, IIndex index, boolean wait, IProgressMonitor progressMonitor) {
+	private IASTTranslationUnit getAST(ITranslationUnit tUnit, IIndex index, boolean wait,
+			IProgressMonitor progressMonitor) {
 		if (tUnit == null)
 			return null;
 		
@@ -237,29 +231,49 @@ public class ASTCache {
 		}
 	}
 
-	public IASTTranslationUnit acquireSharedAST(ITranslationUnit tUnit, IIndex index, boolean wait,
+	/**
-			IProgressMonitor monitor) {
+	 * Returns a shared AST for the given translation unit and locks it for
-		IASTTranslationUnit ast= getAST(tUnit, index, wait, monitor);
+	 * exclusive access. An AST obtained from this method has to be released
-		if (ast == null)
+	 * by calling {@link #releaseSharedAST(IASTTranslationUnit)}.
-			return null;
+	 * Subsequent call to this method will block until the AST is released.
-		synchronized (fCacheMutex) {
+	 * <p>
-			if (ast == fAST) {
+	 * The AST can be released by a thread other than the one that acquired it.
+	 * <p>
+	 * Clients are not allowed to modify the AST and must hold an index read
+	 * lock prior to calling this method and continue to hold the lock as long
+	 * as the AST is being used.
+	 * </p>
+	 *
+	 * @param tUnit				the translation unit
+	 * @param index				the index used to create the AST, needs to be read-locked
+	 * @param wait				if <code>true</code>, wait for AST to be computed
+	 *                          (might compute a new AST)
+	 * @param progressMonitor	the progress monitor or <code>null</code>
+	 * @return					the AST or <code>null</code> if the AST is not available
+	 */
+	public final IASTTranslationUnit acquireSharedAST(ITranslationUnit tUnit, IIndex index,
+			boolean wait, IProgressMonitor progressMonitor) {
+		IASTTranslationUnit ast= getAST(tUnit, index, wait, progressMonitor);
+		if (ast != null) {
 			try {
-					fASTSemaphore.acquire();
+				((ASTTranslationUnit) ast).beginExclusiveAccess();
 			} catch (InterruptedException e) {
 				throw new OperationCanceledException();
 			}
 		}
-		}
 		return ast;
 	}
 
-	public void releaseSharedAST(IASTTranslationUnit ast) {
+	/**
-		synchronized (fCacheMutex) {
+	 * Releases a shared AST previously acquired by calling
-			if (ast == fAST) {
+	 * {@link #acquireSharedAST(ITranslationUnit, IIndex, boolean, IProgressMonitor)}.
-				fASTSemaphore.release();
+	 * <p>
-			}
+	 * Can be called by a thread other than the one that acquired the AST.
-		}
+	*
+	 * @param ast   the AST to release.
+	 */
+	public final void releaseSharedAST(IASTTranslationUnit ast) {
+		((ASTTranslationUnit) ast).endExclusiveAccess();
 	}
 
 	/**
@@ -283,7 +297,6 @@ public class ASTCache {
 			disposeAST();
 
 		fAST= ast;
-		fASTSemaphore= fAST == null ? null : new Semaphore(1); 
 		fLastWriteOnIndex= fAST == null ? 0 : fAST.getIndex().getLastWriteAccess();
 
 		// Signal AST change
@@ -302,7 +315,6 @@ public class ASTCache {
 				System.out.println(DEBUG_PREFIX + getThreadName() + "disposing AST: " + toString(fAST) + " for: " + toString(fActiveTU)); //$NON-NLS-1$ //$NON-NLS-2$ 
 	
 			fAST= null;
-			fASTSemaphore= null;
 			cache(null, null);
 		}
 	}

core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/ASTTranslationUnit.java

@@ -11,6 +11,7 @@
 package org.eclipse.cdt.internal.core.dom.parser;
 
 import java.util.List;
+import java.util.concurrent.Semaphore;
 
 import org.eclipse.cdt.core.CCorePlugin;
 import org.eclipse.cdt.core.dom.IName;
@@ -70,6 +71,8 @@ public abstract class ASTTranslationUnit extends ASTNode implements IASTTranslat
 	private INodeFactory fNodeFactory;
 	private boolean fForContentAssist;
 	private ITranslationUnit fOriginatingTranslationUnit;
+	/** The semaphore controlling exclusive access to the AST. */
+	private final Semaphore fSemaphore= new Semaphore(1);
 
 	@Override
 	public final IASTTranslationUnit getTranslationUnit() {
@@ -446,4 +449,16 @@ public abstract class ASTTranslationUnit extends ASTNode implements IASTTranslat
 	public void setOriginatingTranslationUnit(ITranslationUnit tu) {
 		this.fOriginatingTranslationUnit = tu;
 	}
+
+	/**
+	 * Starts exclusive access 
+	 * @throws InterruptedException
+	 */
+	public void beginExclusiveAccess() throws InterruptedException {
+		fSemaphore.acquire();
+	}
+
+	public void endExclusiveAccess() {
+		fSemaphore.release();
+	}
 }

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/ASTProvider.java

@@ -31,10 +31,12 @@ import org.eclipse.ui.texteditor.ITextEditor;
 import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
 import org.eclipse.cdt.core.index.IIndex;
 import org.eclipse.cdt.core.model.ICElement;
+import org.eclipse.cdt.core.model.ILanguage;
 import org.eclipse.cdt.core.model.ITranslationUnit;
 import org.eclipse.cdt.ui.CUIPlugin;
 
 import org.eclipse.cdt.internal.core.model.ASTCache;
+import org.eclipse.cdt.internal.core.model.ASTCache.ASTRunnable;
 
 /**
  * Provides a shared AST for clients. The shared AST is
@@ -325,11 +327,22 @@ public final class ASTProvider {
 		fCache.reconciled(ast, (ITranslationUnit) cElement);
 	}
 
+	/**
+	 * Executes {@link ASTRunnable#runOnAST(ILanguage, IASTTranslationUnit)}
+	 * with the shared AST for the given translation unit. Handles acquiring
+	 * and releasing the index read-lock for the client.
+	 * 
+	 * @param cElement     the translation unit
+	 * @param waitFlag     condition for waiting for the AST to be built.
+	 * @param monitor      a progress monitor, may be <code>null</code>
+	 * @param astRunnable  the runnable taking the AST
+	 * @return the status  returned by the ASTRunnable
+	 */
 	public IStatus runOnAST(ICElement cElement, WAIT_FLAG waitFlag, IProgressMonitor monitor,
 			ASTCache.ASTRunnable astRunnable) {
 		Assert.isTrue(cElement instanceof ITranslationUnit);
 		final ITranslationUnit tu = (ITranslationUnit) cElement;
-		if (!canUseCache(tu, waitFlag))
+		if (!prepareForUsingCache(tu, waitFlag))
 			return Status.CANCEL_STATUS;
 		return fCache.runOnAST(tu, waitFlag != WAIT_NO, monitor, astRunnable);
 	}
@@ -352,7 +365,7 @@ public final class ASTProvider {
 	 */
 	public final IASTTranslationUnit acquireSharedAST(ITranslationUnit tu, IIndex index,
 			WAIT_FLAG waitFlag, IProgressMonitor monitor) {
-		if (!canUseCache(tu, waitFlag))
+		if (!prepareForUsingCache(tu, waitFlag))
 			return null;
 		return fCache.acquireSharedAST(tu, index, waitFlag != WAIT_NO, monitor);
 	}
@@ -369,7 +382,15 @@ public final class ASTProvider {
 		fCache.releaseSharedAST(ast);
 	}
 
-	private synchronized boolean canUseCache(ITranslationUnit tu, WAIT_FLAG waitFlag) {
+	/**
+	 * Prepares the AST cache to be used for the given translation unit.
+	 * 
+	 * @param tu the translation unit.
+	 * @param waitFlag condition for waiting for the AST to be built.
+	 * @return <code>true</code> if the AST cache can be used for the given translation unit,
+	 * 		<code>false</code> otherwise.
+	 */
+	private boolean prepareForUsingCache(ITranslationUnit tu, WAIT_FLAG waitFlag) {
 		final boolean isActive= fCache.isActiveElement(tu);
 		if (!tu.isOpen())
 			return false;