Bug 459186. Added safeguards against misuse of content assist contexts

core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/contentassist2/AbstractContentAssistTest.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2004, 2014 IBM Corporation and others.
+ * Copyright (c) 2004, 2015 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -11,6 +11,7 @@
  *     Bryan Wilkinson (QNX)
  *     Markus Schorn (Wind River Systems)
  *     Thomas Corbat (IFS)
+ *     Sergey Prigogin (Google)
  *******************************************************************************/
 package org.eclipse.cdt.ui.tests.text.contentassist2;
 
@@ -39,6 +40,7 @@ import org.eclipse.cdt.ui.testplugin.EditorTestHelper;
 import org.eclipse.cdt.ui.tests.BaseUITestCase;
 import org.eclipse.cdt.ui.text.ICCompletionProposal;
 import org.eclipse.cdt.ui.text.ICPartitions;
+import org.eclipse.cdt.ui.text.contentassist.ContentAssistInvocationContext;
 
 import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPASTNameBase;
 
@@ -72,23 +74,23 @@ public abstract class AbstractContentAssistTest extends BaseUITestCase {
 		fCFile= setUpProjectContent(fCProject.getProject());
 		assertNotNull(fCFile);
 		waitForIndexer(fCProject);
-		fEditor= (ITextEditor)EditorTestHelper.openInEditor(fCFile, true);
+		fEditor= (ITextEditor) EditorTestHelper.openInEditor(fCFile, true);
 		assertNotNull(fEditor);
 		CPPASTNameBase.sAllowNameComputation= true;
 
-//		EditorTestHelper.joinBackgroundActivities((AbstractTextEditor)fEditor);
+//		EditorTestHelper.joinBackgroundActivities((AbstractTextEditor) fEditor);
 	}
 
 	/**
 	 * Setup the project's content.
 	 * @param project
 	 * @return  the file to be opened in the editor
-	 * @throws Exception 
 	 */
 	protected abstract IFile setUpProjectContent(IProject project) throws Exception;
 
 	@Override
 	protected void tearDown() throws Exception {
+		ContentAssistInvocationContext.assertNoUndisposedContexts();
 		EditorTestHelper.closeEditor(fEditor);
 		fEditor= null;
 		CProjectHelper.delete(fCProject);
@@ -97,12 +99,13 @@ public abstract class AbstractContentAssistTest extends BaseUITestCase {
 		super.tearDown();
 	}
 
-	protected void assertContentAssistResults(int offset, int length, String[] expected, boolean isCompletion, boolean isTemplate, boolean filterResults, CompareType compareType) throws Exception {
+	protected void assertContentAssistResults(int offset, int length, String[] expected,
+			boolean isCompletion, boolean isTemplate, boolean filterResults, CompareType compareType) throws Exception {
 		if (CTestPlugin.getDefault().isDebugging())  {
-			System.out.println("\n\n\n\n\nTesting "+this.getClass().getName());
+			System.out.println("\n\n\n\n\nTesting " + this.getClass().getName());
 		}
 
-		//Call the CContentAssistProcessor
+		// Call the CContentAssistProcessor
 		ISourceViewer sourceViewer= EditorTestHelper.getSourceViewer((AbstractTextEditor)fEditor);
 		String contentType= TextUtilities.getContentType(sourceViewer.getDocument(), ICPartitions.C_PARTITIONING, offset, true);
 		boolean isCode= IDocument.DEFAULT_CONTENT_TYPE.equals(contentType);
@@ -288,8 +291,6 @@ public abstract class AbstractContentAssistTest extends BaseUITestCase {
 		return EditorTestHelper.getDocument(fEditor);
 	}
 
-
-
 	protected void setCommaAfterFunctionParameter(String value) {
 		fCProject.setOption(
 						DefaultCodeFormatterConstants.FORMATTER_INSERT_SPACE_AFTER_COMMA_IN_METHOD_DECLARATION_PARAMETERS, value);

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/text/contentassist/CContentAssistInvocationContext.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2005, 2014 IBM Corporation and others.
+ * Copyright (c) 2005, 2015 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -10,6 +10,7 @@
  *     Anton Leherbauer (Wind River Systems)
  *     Bryan Wilkinson (QNX)
  *     Thomas Corbat (IFS)
+ *     Sergey Prigogin (Google)
  *******************************************************************************/
 package org.eclipse.cdt.internal.ui.text.contentassist;
 
@@ -41,16 +42,18 @@ import org.eclipse.cdt.internal.ui.text.Symbols;
 /**
  * Describes the context of a content assist invocation in a C/C++ editor.
  * <p>
- * Clients may use but not subclass this class.
+ * Clients may instantiate. A client that created a context is responsible for its disposal.
  * </p>
  * 
  * @since 4.0
  */
-public class CContentAssistInvocationContext extends ContentAssistInvocationContext implements ICEditorContentAssistInvocationContext {
+public class CContentAssistInvocationContext extends ContentAssistInvocationContext
+		implements ICEditorContentAssistInvocationContext {
 	private final IEditorPart fEditor;
 	private final boolean fIsCompletion;
 	private final boolean fIsAutoActivated;
-	private IIndex fIndex = null;
+	private IIndex fIndex;
+
 	private Lazy<Integer> fContextInfoPosition = new Lazy<Integer>() {
 		@Override
 		protected Integer calculateValue() {
@@ -247,6 +250,7 @@ public class CContentAssistInvocationContext extends ContentAssistInvocationCont
 	 */
 	@Override
 	public ITranslationUnit getTranslationUnit() {
+		assertNotDisposed();
 		return fTU.value();
 	}
 	
@@ -258,12 +262,14 @@ public class CContentAssistInvocationContext extends ContentAssistInvocationCont
 	 */
 	@Override
 	public ICProject getProject() {
+		assertNotDisposed();
 		ITranslationUnit unit= getTranslationUnit();
 		return unit == null ? null : unit.getCProject();
 	}
 		
 	@Override
 	public IASTCompletionNode getCompletionNode() {
+		assertNotDisposed();
 		// For scalability.
 		if (fEditor != null && fEditor instanceof CEditor) {
 			CEditor editor = (CEditor) fEditor;
@@ -287,6 +293,7 @@ public class CContentAssistInvocationContext extends ContentAssistInvocationCont
 	
 	@Override
 	public int getParseOffset() {
+		assertNotDisposed();
 		return fParseOffset.value();
 	}
 
@@ -295,6 +302,7 @@ public class CContentAssistInvocationContext extends ContentAssistInvocationCont
 	 */
 	@Override
 	public int getContextInformationOffset() {
+		assertNotDisposed();
 		return fContextInfoPosition.value();
 	}
 	
@@ -306,6 +314,7 @@ public class CContentAssistInvocationContext extends ContentAssistInvocationCont
 	 * @return a sensible completion offset
 	 */
 	protected int guessCompletionPosition(int contextPosition) {
+		assertNotDisposed();
 		CHeuristicScanner scanner= new CHeuristicScanner(getDocument());
 		int bound= Math.max(-1, contextPosition - 200);
 		
@@ -354,6 +363,7 @@ public class CContentAssistInvocationContext extends ContentAssistInvocationCont
 	 *     offset is not inside a function call (or similar)
 	 */
 	protected int guessContextInformationPosition() {
+		assertNotDisposed();
 		final int contextPosition= getInvocationOffset();
 		
 		CHeuristicScanner scanner= new CHeuristicScanner(getDocument());
@@ -386,15 +396,18 @@ public class CContentAssistInvocationContext extends ContentAssistInvocationCont
 	 */
 	@Override
 	public IEditorPart getEditor() {
+		assertNotDisposed();
 		return fEditor;
 	}
 
 	@Override
 	public boolean isContextInformationStyle() {
+		assertNotDisposed();
 		return !fIsCompletion || (getParseOffset() != getInvocationOffset());
 	}
 	
 	public boolean isAutoActivated() {
+		assertNotDisposed();
 		return fIsAutoActivated;
 	}
 
@@ -402,31 +415,38 @@ public class CContentAssistInvocationContext extends ContentAssistInvocationCont
 	public void dispose() {
 		if (fIndex != null) {
 			fIndex.releaseReadLock();
+			fIndex = null;
 		}
 		super.dispose();
 	}
 
 	public boolean isAfterOpeningParenthesis() {
+		assertNotDisposed();
 		return afterOpeningParenthesis.value();
 	}
 
 	public boolean isAfterOpeningAngleBracket() {
+		assertNotDisposed();
 		return afterOpeningAngleBracket.value();
 	}
 
 	public boolean isInUsingDirective() {
+		assertNotDisposed();
 		return inUsingDeclaration.value();
 	}
 
 	public boolean isFollowedBySemicolon() {
+		assertNotDisposed();
 		return followedBySemicolon.value();
 	}
 
 	public String getFunctionParameterDelimiter() {
+		assertNotDisposed();
 		return functionParameterDelimiter.value();
 	}
 
 	public String getTemplateParameterDelimiter() {
+		assertNotDisposed();
 		return templateParameterDelimiter.value();
 	}
 }

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/text/contentassist/ContentAssistInvocationContext.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- *  Copyright (c) 2005, 2009 IBM Corporation and others.
+ *  Copyright (c) 2005, 2015 IBM Corporation and others.
  *  All rights reserved. This program and the accompanying materials
  *  are made available under the terms of the Eclipse Public License v1.0
  *  which accompanies this distribution, and is available at
@@ -9,9 +9,12 @@
  *     IBM Corporation - initial API and implementation
  *     Anton Leherbauer (Wind River Systems)
  *     Bryan Wilkinson (QNX)
+ *     Sergey Prigogin (Google)
  *******************************************************************************/
 package org.eclipse.cdt.ui.text.contentassist;
 
+import java.util.concurrent.atomic.AtomicInteger;
+
 import org.eclipse.core.runtime.Assert;
 import org.eclipse.jface.text.BadLocationException;
 import org.eclipse.jface.text.IDocument;
@@ -26,20 +29,32 @@ import org.eclipse.jface.text.ITextViewer;
  * specific context information such as an AST.
  * </p>
  * <p>
- * Clients may instantiate. Any created context has to be disposed.
+ * Clients may instantiate. A client that created a context is responsible for its disposal.
  * </p>
  * @noextend This class is not intended to be subclassed by clients.
  * @since 4.0
  */
 public class ContentAssistInvocationContext {
-	/* state */
+	private static final AtomicInteger numberOfUndisposedContexts = new AtomicInteger();
+
+	/* State. */
 	private final ITextViewer fViewer;
 	private final IDocument fDocument;
 	private final int fOffset;
-	
+	private boolean fDisposed;
-	/* cached additional info */
+
+	/* Cached additional info. */
 	private CharSequence fPrefix;
-	
+
+	/**
+	 * For tests only.
+	 * @since 5.9
+	 */
+	public static void assertNoUndisposedContexts() {
+		Assert.isTrue(numberOfUndisposedContexts.get() == 0,
+				numberOfUndisposedContexts.get() + " ContentAssistInvocationContext objects have not been disposed."); //$NON-NLS-1$
+	}
+
 	/**
 	 * Equivalent to
 	 * {@linkplain #ContentAssistInvocationContext(ITextViewer, int) ContentAssistInvocationContext(viewer, viewer.getSelectedRange().x)}.
@@ -61,6 +76,7 @@ public class ContentAssistInvocationContext {
 		fViewer= viewer;
 		fDocument= null;
 		fOffset= offset;
+		numberOfUndisposedContexts.incrementAndGet();
 	}
 	
 	/**
@@ -70,6 +86,7 @@ public class ContentAssistInvocationContext {
 		fDocument= null;
 		fViewer= null;
 		fOffset= -1;
+		numberOfUndisposedContexts.incrementAndGet();
 	}
 	
 	/**
@@ -84,6 +101,7 @@ public class ContentAssistInvocationContext {
 		fViewer= null;
 		fDocument= document;
 		fOffset= offset;
+		numberOfUndisposedContexts.incrementAndGet();
 	}
 	
 	/**
@@ -92,6 +110,7 @@ public class ContentAssistInvocationContext {
 	 * @return the invocation offset
 	 */
 	public final int getInvocationOffset() {
+		assertNotDisposed();
 		return fOffset;
 	}
 	
@@ -101,6 +120,7 @@ public class ContentAssistInvocationContext {
 	 * @return the viewer, possibly <code>null</code>
 	 */
 	public final ITextViewer getViewer() {
+		assertNotDisposed();
 		return fViewer;
 	}
 	
@@ -110,6 +130,7 @@ public class ContentAssistInvocationContext {
 	 * @return the document or <code>null</code>
 	 */
 	public IDocument getDocument() {
+		assertNotDisposed();
 		if (fDocument == null) {
 			if (fViewer == null)
 				return null;
@@ -127,6 +148,7 @@ public class ContentAssistInvocationContext {
 	 * @throws BadLocationException if accessing the document fails
 	 */
 	public CharSequence computeIdentifierPrefix() throws BadLocationException {
+		assertNotDisposed();
 		if (fPrefix == null) {
 			IDocument document= getDocument();
 			if (document == null)
@@ -145,12 +167,23 @@ public class ContentAssistInvocationContext {
 	}
 	
 	/**
-	 * Called upon completion of the content assist. Used to free any resources
+	 * Must be called upon completion of the content assist. Used to free any resources
 	 * used by the context.
 	 */
 	public void dispose() {
+		assertNotDisposed();
+		fDisposed = true;
+		numberOfUndisposedContexts.decrementAndGet();
 	}
-	
+
+	/**
+	 * @since 5.9
+	 */
+	protected void assertNotDisposed() {
+		if (fDisposed)
+			throw new IllegalArgumentException("The content assist context has been disposed already"); //$NON-NLS-1$
+	}
+
 	/**
 	 * Invocation contexts are equal if they describe the same context and are of the same type.
 	 * This implementation checks for <code>null</code> values and class equality. Subclasses
@@ -193,9 +226,6 @@ public class ContentAssistInvocationContext {
 		return (fViewer == null && other.fViewer == null || fViewer != null && fViewer.equals(other.fViewer)) && fOffset == other.fOffset && (fDocument == null && other.fDocument == null || fDocument != null && fDocument.equals(other.fDocument));
 	}
 	
-	/*
-	 * @see java.lang.Object#hashCode()
-	 */
 	@Override
 	public int hashCode() {
 		return 23459213 << 5 | (fViewer == null ? 0 : fViewer.hashCode() << 3) | fOffset;