From 3cbb64387018f9b2e15cefd2807cd1c2dc7a6de2 Mon Sep 17 00:00:00 2001 From: Sergey Prigogin Date: Fri, 6 Feb 2015 15:25:09 -0800 Subject: [PATCH] Bug 459186. Added safeguards against misuse of content assist contexts --- .../AbstractContentAssistTest.java | 19 +++---- .../CContentAssistInvocationContext.java | 28 ++++++++-- .../ContentAssistInvocationContext.java | 52 +++++++++++++++---- 3 files changed, 75 insertions(+), 24 deletions(-) diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/contentassist2/AbstractContentAssistTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/contentassist2/AbstractContentAssistTest.java index bc3ab7236b0..8f2b49554b1 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/text/contentassist2/AbstractContentAssistTest.java +++ b/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); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/text/contentassist/CContentAssistInvocationContext.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/text/contentassist/CContentAssistInvocationContext.java index 17f6de74609..87a2abda2dc 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/text/contentassist/CContentAssistInvocationContext.java +++ b/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. *

- * Clients may use but not subclass this class. + * Clients may instantiate. A client that created a context is responsible for its disposal. *

* * @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 fContextInfoPosition = new Lazy() { @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(); } } diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/text/contentassist/ContentAssistInvocationContext.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/text/contentassist/ContentAssistInvocationContext.java index c1e905fd191..6d612441fe5 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/text/contentassist/ContentAssistInvocationContext.java +++ b/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. *

*

- * Clients may instantiate. Any created context has to be disposed. + * Clients may instantiate. A client that created a context is responsible for its disposal. *

* @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; - - /* cached additional info */ + private boolean fDisposed; + + /* 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 null */ public final ITextViewer getViewer() { + assertNotDisposed(); return fViewer; } @@ -110,6 +130,7 @@ public class ContentAssistInvocationContext { * @return the document or null */ 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 null 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;