From 35930ec626ac614b229648bdd2ada8f7e42a8891 Mon Sep 17 00:00:00 2001 From: Jason Malinowski Date: Thu, 6 Feb 2020 17:04:00 -0800 Subject: [PATCH] Unwrap SoftCrashExceptions so we can actually deal with the failure Calls to our remote process are wrapped in a "SoftCrashException" in an attempt to prevent crashes; this exception derives from OperationCanceledException. This backfires: our GraphQueryManager does support reporting exceptions to an info bar, so this means anything that goes wrong in the RPC then gets silently swallowed because we think it's cancellation instead. Unwrapping makes things better again. This commit should be reverted when we remove SoftCrashException in https://github.com/dotnet/roslyn/issues/40476. --- .../GraphQueries/SearchGraphQuery.cs | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/VisualStudio/Core/Def/Implementation/Progression/GraphQueries/SearchGraphQuery.cs b/src/VisualStudio/Core/Def/Implementation/Progression/GraphQueries/SearchGraphQuery.cs index d139c8c65bc..5833109723a 100644 --- a/src/VisualStudio/Core/Def/Implementation/Progression/GraphQueries/SearchGraphQuery.cs +++ b/src/VisualStudio/Core/Def/Implementation/Progression/GraphQueries/SearchGraphQuery.cs @@ -13,6 +13,7 @@ using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.VisualStudio.GraphModel; using Roslyn.Utilities; +using System.Runtime.ExceptionServices; namespace Microsoft.VisualStudio.LanguageServices.Implementation.Progression { @@ -109,8 +110,28 @@ private async Task AddLinkedNodeForMemberAsync(Project project, ISymb internal async Task> FindNavigableSourceSymbolsAsync( Project project, CancellationToken cancellationToken) { - var declarations = await DeclarationFinder.FindSourceDeclarationsWithPatternAsync( - project, _searchPattern, SymbolFilter.TypeAndMember, cancellationToken).ConfigureAwait(false); + ImmutableArray declarations; + + // FindSourceDeclarationsWithPatternAsync calls into OOP to do the search; if something goes badly it + // throws a SoftCrashException which inherits from OperationCanceledException. This is unfortunate, because + // it means that other bits of code see this as a cancellation and then may crash because they expect that if this + // method is raising cancellation, it's because cancellationToken requested the cancellation. The intent behind + // SoftCrashException was since it inherited from OperationCancelled it would make things safer, but in this case + // it's violating other invariants in the process which creates other problems. + // + // https://github.com/dotnet/roslyn/issues/40476 tracks removing SoftCrashException. When it is removed, the + // catch here can be removed and simply let the exception propagate; our Progression code is hardened to + // handle exceptions and report them gracefully. + try + { + declarations = await DeclarationFinder.FindSourceDeclarationsWithPatternAsync( + project, _searchPattern, SymbolFilter.TypeAndMember, cancellationToken).ConfigureAwait(false); + } + catch (SoftCrashException ex) when (ex.InnerException != null) + { + ExceptionDispatchInfo.Capture(ex.InnerException).Throw(); + throw ExceptionUtilities.Unreachable; + } var symbols = declarations.SelectAsArray(d => d.Symbol); -- GitLab