From 80ca2e52aa016578c868e79739823deefd49247a Mon Sep 17 00:00:00 2001 From: Sergey Prigogin Date: Thu, 22 Aug 2013 09:43:51 -0700 Subject: [PATCH] Bug 415702 - Invalid thread access in IncludeCreator.findContribution method --- .../internal/ui/editor/AddIncludeAction.java | 13 ++-- .../refactoring/includes/IncludeCreator.java | 75 +++++++------------ .../ui/refactoring/includes/Messages.java | 2 - .../refactoring/includes/Messages.properties | 2 - 4 files changed, 34 insertions(+), 58 deletions(-) diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/AddIncludeAction.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/AddIncludeAction.java index 376a6f91d61..caa1ec19e52 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/AddIncludeAction.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/AddIncludeAction.java @@ -10,9 +10,7 @@ *******************************************************************************/ package org.eclipse.cdt.internal.ui.editor; -import java.util.ArrayList; import java.util.Collection; -import java.util.List; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IStatus; @@ -28,7 +26,6 @@ import org.eclipse.swt.widgets.Display; import org.eclipse.swt.widgets.Shell; import org.eclipse.text.edits.MalformedTreeException; import org.eclipse.text.edits.MultiTextEdit; -import org.eclipse.text.edits.TextEdit; import org.eclipse.text.undo.DocumentUndoManagerRegistry; import org.eclipse.text.undo.IDocumentUndoManager; import org.eclipse.ui.IEditorInput; @@ -102,7 +99,7 @@ public class AddIncludeAction extends TextEditorAction { } final String lineDelimiter = getLineDelimiter(editor); - final List edits = new ArrayList(); + final MultiTextEdit[] holder = new MultiTextEdit[1]; SharedASTJob job = new SharedASTJob(CEditorMessages.AddInclude_action, tu) { @Override public IStatus runOnAST(ILanguage lang, IASTTranslationUnit ast) throws CoreException { @@ -111,7 +108,7 @@ public class AddIncludeAction extends TextEditorAction { try { index.acquireReadLock(); IncludeCreator creator = new IncludeCreator(tu, index, lineDelimiter, fAmbiguityResolver); - edits.addAll(creator.createInclude(ast, (ITextSelection) selection)); + holder[0] = creator.createInclude(ast, (ITextSelection) selection); return Status.OK_STATUS; } catch (InterruptedException e) { return Status.CANCEL_STATUS; @@ -122,10 +119,9 @@ public class AddIncludeAction extends TextEditorAction { }; IStatus status = BusyCursorJobRunner.execute(job); if (status.isOK()) { - if (!edits.isEmpty()) { + MultiTextEdit edit = holder[0]; + if (edit.hasChildren()) { // Apply text edits. - MultiTextEdit edit = new MultiTextEdit(); - edit.addChildren(edits.toArray(new TextEdit[edits.size()])); IEditorInput editorInput = editor.getEditorInput(); IDocument document = editor.getDocumentProvider().getDocument(editorInput); IDocumentUndoManager manager= DocumentUndoManagerRegistry.getDocumentUndoManager(document); @@ -175,6 +171,7 @@ public class AddIncludeAction extends TextEditorAction { /** * Returns the translation unit of the given editor. + * * @param editor The editor. * @return The translation unit. */ diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeCreator.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeCreator.java index cc167c73bf0..0e7ff78165a 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeCreator.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludeCreator.java @@ -16,7 +16,6 @@ package org.eclipse.cdt.internal.ui.refactoring.includes; import static org.eclipse.cdt.core.index.IndexLocationFactory.getAbsolutePath; import static org.eclipse.cdt.internal.ui.refactoring.includes.IncludeUtil.isContainedInRegion; -import java.lang.reflect.InvocationTargetException; import java.net.URI; import java.util.ArrayDeque; import java.util.ArrayList; @@ -32,16 +31,13 @@ import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IProject; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IPath; -import org.eclipse.core.runtime.IProgressMonitor; import org.eclipse.core.runtime.NullProgressMonitor; import org.eclipse.core.runtime.Path; import org.eclipse.core.runtime.content.IContentType; -import org.eclipse.jface.operation.IRunnableWithProgress; import org.eclipse.jface.text.IRegion; import org.eclipse.jface.text.ITextSelection; import org.eclipse.text.edits.InsertEdit; -import org.eclipse.text.edits.TextEdit; -import org.eclipse.ui.PlatformUI; +import org.eclipse.text.edits.MultiTextEdit; import com.ibm.icu.text.Collator; @@ -111,13 +107,14 @@ public class IncludeCreator { fContext = new IncludeCreationContext(tu, index); } - public List createInclude(IASTTranslationUnit ast, ITextSelection selection) + public MultiTextEdit createInclude(IASTTranslationUnit ast, ITextSelection selection) throws CoreException { + MultiTextEdit rootEdit = new MultiTextEdit(); ITranslationUnit tu = fContext.getTranslationUnit(); IASTNodeSelector selector = ast.getNodeSelector(tu.getLocation().toOSString()); IASTName name = selector.findEnclosingName(selection.getOffset(), selection.getLength()); if (name == null) { - return Collections.emptyList(); + return rootEdit; } char[] nameChars = name.toCharArray(); String lookupName = new String(nameChars); @@ -132,7 +129,7 @@ public class IncludeCreator { } } if (nameChars.length == 0) { - return Collections.emptyList(); + return rootEdit; } final Map candidatesMap= new HashMap(); @@ -187,7 +184,7 @@ public class IncludeCreator { if (candidates.size() > 1) { IncludeCandidate candidate = fAmbiguityResolver.selectElement(candidates); if (candidate == null) - return Collections.emptyList(); + return rootEdit; candidates.clear(); candidates.add(candidate); } @@ -206,7 +203,7 @@ public class IncludeCreator { } } catch (CoreException e) { CUIPlugin.log(e); - return Collections.emptyList(); + return rootEdit; } if (requiredIncludes.isEmpty() && !lookupName.isEmpty()) { @@ -226,10 +223,10 @@ public class IncludeCreator { } } - return createEdits(requiredIncludes, usingDeclarations, ast, selection); + return createEdit(requiredIncludes, usingDeclarations, ast, selection); } - private List createEdits(List includes, + private MultiTextEdit createEdit(List includes, List usingDeclarations, IASTTranslationUnit ast, ITextSelection selection) { NodeCommentMap commentedNodeMap = ASTCommenter.getCommentedNodeMap(ast); @@ -239,7 +236,7 @@ public class IncludeCreator { IncludePreferences preferences = fContext.getPreferences(); - List edits = new ArrayList(); + MultiTextEdit rootEdit = new MultiTextEdit(); IASTPreprocessorIncludeStatement[] existingIncludes = ast.getIncludeDirectives(); fContext.addHeadersIncludedPreviously(existingIncludes); @@ -294,7 +291,7 @@ public class IncludeCreator { IASTNode previousNode = previousInclude.getExistingInclude(); if (previousNode != null) { offset = ASTNodes.skipToNextLineAfterNode(contents, previousNode); - flushEditBuffer(offset, text, edits); + flushEditBuffer(offset, text, rootEdit); if (contents[offset - 1] != '\n') text.append(fLineDelimiter); } @@ -308,11 +305,11 @@ public class IncludeCreator { include.getStyle().isBlankLineNeededAfter(previousInclude.getStyle(), preferences.includeStyles)) { text.append(fLineDelimiter); } - flushEditBuffer(offset, text, edits); + flushEditBuffer(offset, text, rootEdit); } previousInclude = include; } - flushEditBuffer(offset, text, edits); + flushEditBuffer(offset, text, rootEdit); List mergedUsingDeclarations = getUsingDeclarations(ast); for (UsingDeclaration usingDeclaration : mergedUsingDeclarations) { @@ -324,7 +321,7 @@ public class IncludeCreator { } if (usingDeclarations.isEmpty()) - return edits; + return rootEdit; List temp = null; for (Iterator iter = mergedUsingDeclarations.iterator(); iter.hasNext();) { @@ -367,7 +364,7 @@ public class IncludeCreator { IASTNode previousNode = previousUsing.existingDeclaration; if (previousNode != null) { offset = ASTNodes.skipToNextLineAfterNode(contents, previousNode); - flushEditBuffer(offset, text, edits); + flushEditBuffer(offset, text, rootEdit); if (contents[offset - 1] != '\n') text.append(fLineDelimiter); } @@ -375,13 +372,13 @@ public class IncludeCreator { text.append(using.composeDirective()); text.append(fLineDelimiter); } else { - flushEditBuffer(offset, text, edits); + flushEditBuffer(offset, text, rootEdit); } previousUsing = using; } - flushEditBuffer(offset, text, edits); + flushEditBuffer(offset, text, rootEdit); - return edits; + return rootEdit; } private List getUsingDeclarations(IASTTranslationUnit ast) { @@ -395,9 +392,9 @@ public class IncludeCreator { return usingDeclarations; } - private void flushEditBuffer(int offset, StringBuilder text, List edits) { + private void flushEditBuffer(int offset, StringBuilder text, MultiTextEdit edit) { if (text.length() != 0) { - edits.add(new InsertEdit(offset, text.toString())); + edit.addChild(new InsertEdit(offset, text.toString())); text.delete(0, text.length()); } } @@ -570,33 +567,19 @@ public class IncludeCreator { } private IFunctionSummary findContribution(final String name) throws CoreException { - final IFunctionSummary[] fs = new IFunctionSummary[1]; - IRunnableWithProgress op = new IRunnableWithProgress() { + ICHelpInvocationContext context = new ICHelpInvocationContext() { @Override - public void run(IProgressMonitor monitor) throws InvocationTargetException, InterruptedException { - ICHelpInvocationContext context = new ICHelpInvocationContext() { - @Override - public IProject getProject() { - return fContext.getProject(); - } + public IProject getProject() { + return fContext.getProject(); + } - @Override - public ITranslationUnit getTranslationUnit() { - return fContext.getTranslationUnit(); - } - }; - - fs[0] = CHelpProviderManager.getDefault().getFunctionInfo(context, name); + @Override + public ITranslationUnit getTranslationUnit() { + return fContext.getTranslationUnit(); } }; - try { - PlatformUI.getWorkbench().getProgressService().busyCursorWhile(op); - } catch (InvocationTargetException e) { - throw new CoreException(CUIPlugin.createErrorStatus(Messages.AddInclude_help_provider_error, e)); - } catch (InterruptedException e) { - // Do nothing. Operation has been canceled. - } - return fs[0]; + + return CHelpProviderManager.getDefault().getFunctionInfo(context, name); } /** diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/Messages.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/Messages.java index 8a235ac0d59..ce0348dbfe6 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/Messages.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/Messages.java @@ -13,8 +13,6 @@ package org.eclipse.cdt.internal.ui.refactoring.includes; import org.eclipse.osgi.util.NLS; public final class Messages extends NLS { - public static String AddInclude_help_provider_error; - public static String GCCHeaderSubstitutionMaps_c_map; public static String GCCHeaderSubstitutionMaps_cpp_map; diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/Messages.properties b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/Messages.properties index 7ef265474ab..e4c80e304cd 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/Messages.properties +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/Messages.properties @@ -9,7 +9,5 @@ # Sergey Prigogin (Google) - initial API and implementation ############################################################################### -AddInclude_help_provider_error=Help provider error - GCCHeaderSubstitutionMaps_c_map=GCC C Header Substitution GCCHeaderSubstitutionMaps_cpp_map=GCC C++ Header Substitution