Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSergey Prigogin2011-04-12 22:13:46 +0000
committerSergey Prigogin2011-04-12 22:13:46 +0000
commit87ae09ee72282f39509436d1e59529ac0356a7bb (patch)
tree243ac446d59aad12f5c59c6060084d53c9eaabc1 /codan/org.eclipse.cdt.codan.core.cxx
parentc24ce74075b49689a30a655692de8809a2c574db (diff)
downloadorg.eclipse.cdt-87ae09ee72282f39509436d1e59529ac0356a7bb.tar.gz
org.eclipse.cdt-87ae09ee72282f39509436d1e59529ac0356a7bb.tar.xz
org.eclipse.cdt-87ae09ee72282f39509436d1e59529ac0356a7bb.zip
Bug 337486 - AbstractIndexAstChecker allows AST to outlive index read lock.
Diffstat (limited to 'codan/org.eclipse.cdt.codan.core.cxx')
-rw-r--r--codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/model/AbstractIndexAstChecker.java182
-rw-r--r--codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/model/CxxModelsCache.java158
2 files changed, 178 insertions, 162 deletions
diff --git a/codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/model/AbstractIndexAstChecker.java b/codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/model/AbstractIndexAstChecker.java
index b1874505c51..2a99930285b 100644
--- a/codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/model/AbstractIndexAstChecker.java
+++ b/codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/model/AbstractIndexAstChecker.java
@@ -6,13 +6,14 @@
* http://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
- * Alena Laskavaia - initial API and implementation
+ * Alena Laskavaia - initial API and implementation
+ * Sergey Prigogin (Google)
*******************************************************************************/
package org.eclipse.cdt.codan.core.cxx.model;
-import org.eclipse.cdt.codan.core.CodanCorePlugin;
import org.eclipse.cdt.codan.core.cxx.Activator;
import org.eclipse.cdt.codan.core.model.AbstractCheckerWithProblemPreferences;
+import org.eclipse.cdt.codan.core.model.ICheckerInvocationContext;
import org.eclipse.cdt.codan.core.model.IProblem;
import org.eclipse.cdt.codan.core.model.IProblemLocation;
import org.eclipse.cdt.codan.core.model.IProblemLocationFactory;
@@ -23,95 +24,112 @@ import org.eclipse.cdt.core.dom.ast.IASTMacroExpansionLocation;
import org.eclipse.cdt.core.dom.ast.IASTName;
import org.eclipse.cdt.core.dom.ast.IASTNode;
import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
-import org.eclipse.cdt.core.index.IIndex;
-import org.eclipse.cdt.internal.core.resources.ResourceLookup;
+import org.eclipse.cdt.core.model.CoreModel;
+import org.eclipse.cdt.core.model.ICElement;
+import org.eclipse.cdt.core.model.ITranslationUnit;
import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.IResource;
import org.eclipse.core.runtime.CoreException;
-import org.eclipse.core.runtime.IPath;
-import org.eclipse.core.runtime.Path;
+import org.eclipse.core.runtime.OperationCanceledException;
/**
- * Convenience implementation of checker that work on index based ast of a c/c++
+ * Convenience implementation of checker that works on index-based AST of a C/C++
* program.
*
* Clients may extend this class.
*/
-public abstract class AbstractIndexAstChecker extends AbstractCheckerWithProblemPreferences implements ICAstChecker,
- IRunnableInEditorChecker {
- private IFile file;
- private ICodanCommentMap commentmap;
+public abstract class AbstractIndexAstChecker extends AbstractCheckerWithProblemPreferences
+ implements ICAstChecker, IRunnableInEditorChecker {
+ private CxxModelsCache modelCache;
- protected IFile getFile() {
- return file;
+ @Override
+ public synchronized boolean processResource(IResource resource) throws OperationCanceledException {
+ if (!shouldProduceProblems(resource))
+ return false;
+ if (!(resource instanceof IFile))
+ return true;
+
+ processFile((IFile) resource);
+ return false;
}
- protected IProject getProject() {
- return file == null ? null : file.getProject();
- }
+ private void processFile(IFile file) throws OperationCanceledException {
+ ICheckerInvocationContext context = getContext();
+ synchronized (context) {
+ modelCache = context.get(CxxModelsCache.class);
+ if (modelCache == null) {
+ ICElement celement = CoreModel.getDefault().create(file);
+ if (!(celement instanceof ITranslationUnit)) {
+ return;
+ }
+ modelCache = new CxxModelsCache((ITranslationUnit) celement);
+ context.add(modelCache);
+ }
+ }
- void processFile(IFile file) throws CoreException, InterruptedException {
- commentmap = null;
- IASTTranslationUnit ast = CxxModelsCache.getInstance().getAst(file);
- if (ast == null)
- return;
- // lock the index for read access
- IIndex index = CxxModelsCache.getInstance().getIndex(file);
- index.acquireReadLock();
try {
- // traverse the ast using the visitor pattern.
- this.file = file;
- processAst(ast);
+ IASTTranslationUnit ast = modelCache.getAST();
+ if (ast != null) {
+ synchronized (ast) {
+ processAst(ast);
+ }
+ }
+ } catch (CoreException e) {
+ Activator.log(e);
} finally {
- this.file = null;
- index.releaseReadLock();
+ modelCache = null;
}
}
- public synchronized boolean processResource(IResource resource) {
- if (!shouldProduceProblems(resource))
- return false;
- if (resource instanceof IFile) {
- IFile file = (IFile) resource;
+ /*
+ * (non-Javadoc)
+ *
+ * @see IRunnableInEditorChecker#processModel(Object, ICheckerInvocationContext)
+ */
+ public synchronized void processModel(Object model, ICheckerInvocationContext context) {
+ if (model instanceof IASTTranslationUnit) {
+ setContext(context);
+ IASTTranslationUnit ast = (IASTTranslationUnit) model;
+ synchronized (context) {
+ modelCache = context.get(CxxModelsCache.class);
+ if (modelCache == null) {
+ modelCache = new CxxModelsCache(ast);
+ context.add(modelCache);
+ }
+ }
try {
- processFile(file);
- } catch (CoreException e) {
- CodanCorePlugin.log(e);
- } catch (InterruptedException e) {
- // ignore
+ processAst(ast);
+ } finally {
+ modelCache = null;
+ setContext(null);
}
- return false;
}
- return true;
}
+ @Override
+ public boolean runInEditor() {
+ return true;
+ }
public void reportProblem(String id, IASTNode astNode, Object... args) {
IProblemLocation loc = getProblemLocation(astNode);
- if (loc!=null) reportProblem(id, loc, args);
+ if (loc != null)
+ reportProblem(id, loc, args);
}
+
public void reportProblem(IProblem problem, IASTNode astNode, Object... args) {
IProblemLocation loc = getProblemLocation(astNode);
- if (loc!=null) reportProblem(problem, loc, args);
+ if (loc != null)
+ reportProblem(problem, loc, args);
}
- @SuppressWarnings("restriction")
protected IProblemLocation getProblemLocation(IASTNode astNode) {
IASTFileLocation astLocation = astNode.getFileLocation();
- IPath location = new Path(astLocation.getFileName());
- IFile astFile = ResourceLookup.selectFileForLocation(location, getProject());
- if (astFile == null) {
- astFile = file;
- }
- if (astFile == null) {
- Activator.log("Cannot resolve location: " + location); //$NON-NLS-1$
- return null;
- }
- return getProblemLocation(astNode, astLocation, astFile);
+ return getProblemLocation(astNode, astLocation);
}
- private IProblemLocation getProblemLocation(IASTNode astNode, IASTFileLocation astLocation, IFile astFile) {
+ private IProblemLocation getProblemLocation(IASTNode astNode, IASTFileLocation astLocation) {
int line = astLocation.getStartingLineNumber();
IProblemLocationFactory locFactory = getRuntime().getProblemLocationFactory();
if (hasMacroLocation(astNode) && astNode instanceof IASTName) {
@@ -119,59 +137,35 @@ public abstract class AbstractIndexAstChecker extends AbstractCheckerWithProblem
if (imageLocation != null) {
int start = imageLocation.getNodeOffset();
int end = start + imageLocation.getNodeLength();
- return locFactory.createProblemLocation(astFile, start, end, line);
+ return locFactory.createProblemLocation(getFile(), start, end, line);
}
}
if (line == astLocation.getEndingLineNumber()) {
- return locFactory.createProblemLocation(astFile, astLocation.getNodeOffset(),
+ return locFactory.createProblemLocation(getFile(), astLocation.getNodeOffset(),
astLocation.getNodeOffset() + astLocation.getNodeLength(), line);
}
- return locFactory.createProblemLocation(astFile, line);
+ return locFactory.createProblemLocation(getFile(), line);
}
private boolean hasMacroLocation(IASTNode astNode) {
- return astNode.getNodeLocations().length == 1 && astNode.getNodeLocations()[0] instanceof IASTMacroExpansionLocation;
+ return astNode.getNodeLocations().length == 1 &&
+ astNode.getNodeLocations()[0] instanceof IASTMacroExpansionLocation;
}
- @Override
- public boolean runInEditor() {
- return true;
+ protected IFile getFile() {
+ return modelCache.getFile();
}
- /*
- * (non-Javadoc)
- *
- * @see
- * org.eclipse.cdt.codan.core.model.IRunnableInEditorChecker#processModel
- * (java.lang.Object)
- */
- @SuppressWarnings("restriction")
- public synchronized void processModel(Object model) {
- if (model instanceof IASTTranslationUnit) {
- CxxModelsCache.getInstance().clearCash();
- IASTTranslationUnit ast = (IASTTranslationUnit) model;
- IPath location = new Path(ast.getFilePath());
- IFile astFile = ResourceLookup.selectFileForLocation(location, getProject());
- file = astFile;
- commentmap = null;
- processAst(ast);
- }
+ protected IProject getProject() {
+ IFile file = getFile();
+ return file == null ? null : file.getProject();
}
- protected ICodanCommentMap getCommentMap() {
- if (commentmap == null) {
- try {
- CxxModelsCache cxxcache = CxxModelsCache.getInstance();
- synchronized (cxxcache) {
- IASTTranslationUnit ast = cxxcache.getAst(getFile());
- commentmap = cxxcache.getCommentedNodeMap(ast);
- return commentmap;
- }
+ protected CxxModelsCache getModelCache() {
+ return modelCache;
+ }
- } catch (Exception e) {
- Activator.log(e);
- }
- }
- return commentmap;
+ protected ICodanCommentMap getCommentMap() {
+ return modelCache.getCommentedNodeMap();
}
}
diff --git a/codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/model/CxxModelsCache.java b/codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/model/CxxModelsCache.java
index 6bf238782e6..63bf6095617 100644
--- a/codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/model/CxxModelsCache.java
+++ b/codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/model/CxxModelsCache.java
@@ -6,7 +6,8 @@
* http://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
- * Alena Laskavaia - initial API and implementation
+ * Alena Laskavaia - initial API and implementation
+ * Sergey Prigogin (Google)
*******************************************************************************/
package org.eclipse.cdt.codan.core.cxx.model;
@@ -15,32 +16,71 @@ import java.util.WeakHashMap;
import org.eclipse.cdt.codan.core.cxx.Activator;
import org.eclipse.cdt.codan.core.cxx.internal.model.CodanCommentMap;
import org.eclipse.cdt.codan.core.cxx.internal.model.cfg.CxxControlFlowGraph;
+import org.eclipse.cdt.codan.core.model.ICodanDisposable;
import org.eclipse.cdt.codan.core.model.cfg.IControlFlowGraph;
import org.eclipse.cdt.core.CCorePlugin;
import org.eclipse.cdt.core.dom.ast.IASTFunctionDefinition;
import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
import org.eclipse.cdt.core.index.IIndex;
import org.eclipse.cdt.core.model.CoreModel;
-import org.eclipse.cdt.core.model.ICElement;
+import org.eclipse.cdt.core.model.ICProject;
import org.eclipse.cdt.core.model.ITranslationUnit;
import org.eclipse.cdt.internal.core.dom.rewrite.commenthandler.ASTCommenter;
import org.eclipse.core.resources.IFile;
+import org.eclipse.core.runtime.Assert;
import org.eclipse.core.runtime.CoreException;
+import org.eclipse.core.runtime.OperationCanceledException;
/**
* Cache data models for resource so checkers can share it
*/
-public class CxxModelsCache {
- private IFile file;
+public class CxxModelsCache implements ICodanDisposable {
+ private static final int PARSE_MODE = ITranslationUnit.AST_SKIP_ALL_HEADERS
+ | ITranslationUnit.AST_CONFIGURE_USING_SOURCE_CONTEXT
+ | ITranslationUnit.AST_SKIP_TRIVIAL_EXPRESSIONS_IN_AGGREGATE_INITIALIZERS
+ | ITranslationUnit.AST_PARSE_INACTIVE_CODE;
+
+ private final IFile file;
+ private final ITranslationUnit tu;
private IASTTranslationUnit ast;
- private ITranslationUnit tu;
private IIndex index;
- private WeakHashMap<IASTFunctionDefinition, IControlFlowGraph> cfgmap = new WeakHashMap<IASTFunctionDefinition, IControlFlowGraph>(0);
+ private final WeakHashMap<IASTFunctionDefinition, IControlFlowGraph> cfgmap;
private ICodanCommentMap commentMap;
- private static CxxModelsCache instance = new CxxModelsCache();
+ private boolean disposed;
+
+ CxxModelsCache(ITranslationUnit tu) {
+ this.tu = tu;
+ this.file = tu != null ? (IFile) tu.getResource() : null;
+ cfgmap = new WeakHashMap<IASTFunctionDefinition, IControlFlowGraph>(0);
+ }
+
+ CxxModelsCache(IASTTranslationUnit ast) {
+ this(ast.getOriginatingTranslationUnit());
+ this.ast = ast;
+ }
+
+ public IASTTranslationUnit getAST() throws OperationCanceledException, CoreException {
+ return getAST(tu);
+ }
+
+ public IASTTranslationUnit getAST(ITranslationUnit tu)
+ throws OperationCanceledException, CoreException {
+ if (!this.tu.equals(tu)) {
+ throw new IllegalArgumentException();
+ }
+ if (ast == null) {
+ getIndex();
+ ast= tu.getAST(index, PARSE_MODE);
+ }
+ return ast;
+ }
+
+ public ITranslationUnit getTranslationUnit() {
+ return tu;
+ }
- public static CxxModelsCache getInstance() {
- return instance;
+ public IFile getFile() {
+ return file;
}
public synchronized IControlFlowGraph getControlFlowGraph(IASTFunctionDefinition func) {
@@ -48,85 +88,67 @@ public class CxxModelsCache {
if (cfg != null)
return cfg;
cfg = CxxControlFlowGraph.build(func);
- if (cfgmap.size() > 20) { // if too many function better drop the cash XXX should be LRU
+ // TODO(Alena Laskavaia): Change to LRU.
+ if (cfgmap.size() > 20) { // if too many function better drop the cash
cfgmap.clear();
}
cfgmap.put(func, cfg);
return cfg;
}
- public synchronized IASTTranslationUnit getAst(IFile file) throws CoreException, InterruptedException {
- if (file.equals(this.file)) {
- return ast;
+ public synchronized ICodanCommentMap getCommentedNodeMap() {
+ return getCommentedNodeMap(tu);
+ }
+
+ public synchronized ICodanCommentMap getCommentedNodeMap(ITranslationUnit tu) {
+ if (!this.tu.equals(tu)) {
+ throw new IllegalArgumentException();
}
- // create translation unit and access index
- ICElement celement = CoreModel.getDefault().create(file);
- if (!(celement instanceof ITranslationUnit))
- return null; // not a C/C++ file
- clearCash();
- this.file = file;
- //System.err.println("Making ast for "+file);
- tu = (ITranslationUnit) celement;
- index = CCorePlugin.getIndexManager().getIndex(tu.getCProject());
- // lock the index for read access
- index.acquireReadLock();
- try {
- // create index based ast
- ast = tu.getAST(index, ITranslationUnit.AST_SKIP_INDEXED_HEADERS);
- if (ast == null)
- return null;//
- return ast;
- } finally {
- index.releaseReadLock();
+ if (commentMap == null) {
+ if (ast == null) {
+ throw new IllegalStateException("getCommentedNodeMap called before getAST"); //$NON-NLS-1$
+ }
+ commentMap = new CodanCommentMap(ASTCommenter.getCommentedNodeMap(ast));
}
+ return commentMap;
}
-
- public synchronized ICodanCommentMap getCommentedNodeMap(IASTTranslationUnit ast) {
- if (this.ast == ast) {
+ /**
+ * Returns the index that can be safely used for reading until the cache is disposed.
+ *
+ * @return The index.
+ */
+ public synchronized IIndex getIndex() throws CoreException, OperationCanceledException {
+ Assert.isTrue(!disposed, "CxxASTCache is already disposed."); //$NON-NLS-1$
+ if (this.index == null) {
+ ICProject[] projects = CoreModel.getDefault().getCModel().getCProjects();
+ IIndex index = CCorePlugin.getIndexManager().getIndex(projects);
try {
index.acquireReadLock();
- try {
- commentMap = new CodanCommentMap(ASTCommenter.getCommentedNodeMap(ast));
- } finally {
- index.releaseReadLock();
- }
- return commentMap;
} catch (InterruptedException e) {
- return null;
+ throw new OperationCanceledException();
}
+ this.index = index;
}
- throw new IllegalArgumentException("Not cached");
- }
-
- public ICodanCommentMap getCommentedNodeMap(IFile file) {
- try {
- IASTTranslationUnit ast = getAst(file);
- return getCommentedNodeMap(ast);
- } catch (InterruptedException e) {
- return null;
- } catch (CoreException e) {
- Activator.log(e);
- return null;
- }
+ return this.index;
}
/**
- * Clear cash for current file
+ * @see IDisposable#dispose()
+ * This method should not be called concurrently with any other method.
*/
- public void clearCash() {
- cfgmap.clear();
- ast = null;
- tu = null;
- index = null;
- commentMap = null;
+ public void dispose() {
+ Assert.isTrue(!disposed, "CxxASTCache.dispose() called more than once."); //$NON-NLS-1$
+ disposed = true;
+ if (index != null) {
+ index.releaseReadLock();
+ }
}
- public synchronized IIndex getIndex(IFile file) throws CoreException, InterruptedException {
- if (file.equals(this.file)) {
- return index;
- }
- getAst(file); // to init variables
- return index;
+ @Override
+ protected void finalize() throws Throwable {
+ if (!disposed)
+ Activator.log("CxxASTCache was not disposed."); //$NON-NLS-1$
+ super.finalize();
}
}

Back to the top