Skip to content

Commit

Permalink
Do not crash on unexpected exception (#48367)
Browse files Browse the repository at this point in the history
* Fix usage fo FatalError.Report*

* update handlers in VS and OOP to never fail-fast

* Replace ReportWithoutCrashAndPropagate with Report.

Both APIs propagate the exception. With the previous change ReportWithoutCrash now does not crash VS/OOP.
  • Loading branch information
tmat authored Oct 12, 2020
1 parent 04481d2 commit 9f82a08
Show file tree
Hide file tree
Showing 165 changed files with 352 additions and 432 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ private void VisitNamespaceMembersAsTasks(NamespaceSymbol symbol)
{
Visit(m);
}
catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e))
{
throw ExceptionUtilities.Unreachable;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ private Task CompileNamespaceAsAsync(NamespaceSymbol symbol)
{
CompileNamespace(symbol);
}
catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e))
{
throw ExceptionUtilities.Unreachable;
}
Expand Down Expand Up @@ -397,7 +397,7 @@ private Task CompileNamedTypeAsync(NamedTypeSymbol symbol)
{
CompileNamedType(symbol);
}
catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e))
{
throw ExceptionUtilities.Unreachable;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ protected override TypeSymbol LookupTopLevelTypeDefSymbol(
{
return assembly.LookupTopLevelMetadataType(ref emittedName, digThroughForwardedTypes: true);
}
catch (Exception e) when (FatalError.Report(e)) // Trying to get more useful Watson dumps.
catch (Exception e) when (FatalError.ReportAndPropagate(e)) // Trying to get more useful Watson dumps.
{
throw ExceptionUtilities.Unreachable;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/Core/Portable/Compilation/SemanticModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public SyntaxTree SyntaxTree
{
return GetOperationCore(node, cancellationToken);
}
catch (Exception e) when (FatalError.ReportWithoutCrashUnlessCanceled(e))
catch (Exception e) when (FatalError.ReportAndCatchUnlessCanceled(e))
{
// Log a Non-fatal-watson and then ignore the crash in the attempt of getting operation
Debug.Assert(false, "\n" + e.ToString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public async Task OnCompilationEventsGeneratedAsync(
OnCompilationEventsGenerated_NoLock(getCompilationEvents(eventQueue, additionalFiles));
}
}
catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e))
{
throw ExceptionUtilities.Unreachable;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1407,7 +1407,7 @@ private async Task ProcessCompilationEventsAsync(AnalysisScope analysisScope, An
await ProcessEventAsync(completedEvent, analysisScope, analysisState, cancellationToken).ConfigureAwait(false);
}
}
catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e))
{
throw ExceptionUtilities.Unreachable;
}
Expand Down Expand Up @@ -1466,7 +1466,7 @@ private async Task ProcessCompilationEventsAsync(AnalysisScope analysisScope, An

return completedEvent;
}
catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e))
{
throw ExceptionUtilities.Unreachable;
}
Expand Down Expand Up @@ -2686,7 +2686,7 @@ ImmutableArray<IOperation> getOperationsToAnalyzeWithStackGuard(ImmutableArray<I
{
return GetOperationsToAnalyze(operationBlocksToAnalyze);
}
catch (Exception ex) when (ex is InsufficientExecutionStackException || FatalError.ReportWithoutCrashUnlessCanceled(ex))
catch (Exception ex) when (ex is InsufficientExecutionStackException || FatalError.ReportAndCatchUnlessCanceled(ex))
{
// the exception filter will short-circuit if `ex` is `InsufficientExecutionStackException` (from OperationWalker)
// and no non-fatal-watson will be logged as a result.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1665,7 +1665,7 @@ private static string CreateDisablingMessage(DiagnosticAnalyzer analyzer)
diagnosticIds = diagnosticIds.Add(diagnostic.Id);
}
}
catch (Exception e) when (FatalError.ReportWithoutCrash(e))
catch (Exception e) when (FatalError.ReportAndCatch(e))
{
// Intentionally empty
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,7 @@ private async Task ComputeAnalyzerSyntaxDiagnosticsAsync(AnalysisScope analysisS
await ComputeAnalyzerDiagnosticsAsync(pendingAnalysisScope, getPendingEventsOpt: null, taskToken, cancellationToken).ConfigureAwait(false);
}
}
catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e))
{
throw ExceptionUtilities.Unreachable;
}
Expand Down Expand Up @@ -698,7 +698,7 @@ private async Task ComputeAnalyzerSemanticDiagnosticsAsync(SemanticModel model,
}
}
}
catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e))
{
throw ExceptionUtilities.Unreachable;
}
Expand Down Expand Up @@ -829,7 +829,7 @@ await Task.WhenAll(partialTrees.Select(tree =>
FreeEventQueue(eventQueue, _eventQueuePool);
}
}
catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e))
{
throw ExceptionUtilities.Unreachable;
}
Expand Down Expand Up @@ -868,7 +868,7 @@ await Task.WhenAll(partialTrees.Select(tree =>
FreeDriver(driver);
}
}
catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e))
{
throw ExceptionUtilities.Unreachable;
}
Expand Down Expand Up @@ -962,7 +962,7 @@ private async Task<AnalyzerDriver> GetAnalyzerDriverAsync(CancellationToken canc
}
}
}
catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e))
{
throw ExceptionUtilities.Unreachable;
}
Expand Down Expand Up @@ -1009,7 +1009,7 @@ private async Task ComputeAnalyzerDiagnosticsCoreAsync(AnalyzerDriver driver, As
}
}
}
catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e))
{
throw ExceptionUtilities.Unreachable;
}
Expand Down Expand Up @@ -1137,7 +1137,7 @@ private async Task<Task> SetActiveTreeAnalysisTaskAsync(Func<Tuple<Task, Cancell
await WaitForExecutingTaskAsync(executingTreeTask.Item1).ConfigureAwait(false);
}
}
catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e))
{
throw ExceptionUtilities.Unreachable;
}
Expand Down Expand Up @@ -1344,7 +1344,7 @@ public async Task<AnalyzerTelemetryInfo> GetAnalyzerTelemetryInfoAsync(Diagnosti
var executionTime = GetAnalyzerExecutionTime(analyzer);
return new AnalyzerTelemetryInfo(actionCounts, suppressionActionCounts, executionTime);
}
catch (Exception e) when (FatalError.ReportUnlessCanceled(e))
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e))
{
throw ExceptionUtilities.Unreachable;
}
Expand Down
78 changes: 10 additions & 68 deletions src/Compilers/Core/Portable/InternalUtilities/FatalError.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,14 @@ private static bool IsCurrentOperationBeingCancelled(Exception exception, Cancel
/// </summary>
/// <returns>False to avoid catching the exception.</returns>
[DebuggerHidden]
public static bool ReportUnlessCanceled(Exception exception)
public static bool ReportAndPropagateUnlessCanceled(Exception exception)
{
if (exception is OperationCanceledException)
{
return false;
}

return Report(exception);
return ReportAndPropagate(exception);
}

/// <summary>
Expand All @@ -106,14 +106,14 @@ public static bool ReportUnlessCanceled(Exception exception)
/// </summary>
/// <returns>False to avoid catching the exception.</returns>
[DebuggerHidden]
public static bool ReportUnlessCanceled(Exception exception, CancellationToken cancellationToken)
public static bool ReportAndPropagateUnlessCanceled(Exception exception, CancellationToken cancellationToken)
{
if (IsCurrentOperationBeingCancelled(exception, cancellationToken))
{
return false;
}

return Report(exception);
return ReportAndPropagate(exception);
}

/// <summary>
Expand All @@ -123,14 +123,14 @@ public static bool ReportUnlessCanceled(Exception exception, CancellationToken c
/// </summary>
/// <returns>True to catch the exception.</returns>
[DebuggerHidden]
public static bool ReportWithoutCrashUnlessCanceled(Exception exception)
public static bool ReportAndCatchUnlessCanceled(Exception exception)
{
if (exception is OperationCanceledException)
{
return false;
}

return ReportWithoutCrash(exception);
return ReportAndCatch(exception);
}

/// <summary>
Expand All @@ -140,31 +140,14 @@ public static bool ReportWithoutCrashUnlessCanceled(Exception exception)
/// </summary>
/// <returns>True to catch the exception.</returns>
[DebuggerHidden]
public static bool ReportWithoutCrashUnlessCanceled(Exception exception, CancellationToken cancellationToken)
public static bool ReportAndCatchUnlessCanceled(Exception exception, CancellationToken cancellationToken)
{
if (IsCurrentOperationBeingCancelled(exception, cancellationToken))
{
return false;
}

return ReportWithoutCrash(exception);
}

/// <summary>
/// Use in an exception filter to report a fatal error.
/// Unless the exception is <see cref="NotImplementedException"/>
/// it calls <see cref="Handler"/>. The exception is passed through (the method returns false).
/// </summary>
/// <returns>False to avoid catching the exception.</returns>
[DebuggerHidden]
public static bool ReportUnlessNotImplemented(Exception exception)
{
if (exception is NotImplementedException)
{
return false;
}

return Report(exception);
return ReportAndCatch(exception);
}

/// <summary>
Expand All @@ -173,7 +156,7 @@ public static bool ReportUnlessNotImplemented(Exception exception)
/// </summary>
/// <returns>False to avoid catching the exception.</returns>
[DebuggerHidden]
public static bool Report(Exception exception)
public static bool ReportAndPropagate(Exception exception)
{
Report(exception, s_fatalHandler);
return false;
Expand All @@ -190,53 +173,12 @@ public static bool Report(Exception exception)
/// </summary>
/// <returns>True to catch the exception.</returns>
[DebuggerHidden]
public static bool ReportWithoutCrash(Exception exception)
public static bool ReportAndCatch(Exception exception)
{
Report(exception, s_nonFatalHandler);
return true;
}

/// <summary>
/// Report a non-fatal error like <see cref="ReportWithoutCrash"/> but propagates the exception.
/// </summary>
/// <returns>False to propagate the exception.</returns>
[DebuggerHidden]
public static bool ReportWithoutCrashAndPropagate(Exception exception)
{
Report(exception, s_nonFatalHandler);
return false;
}

/// <summary>
/// Report a non-fatal error like <see cref="ReportWithoutCrash"/> but propagates the exception.
/// </summary>
/// <returns>False to propagate the exception.</returns>
[DebuggerHidden]
public static bool ReportWithoutCrashUnlessCanceledAndPropagate(Exception exception)
{
if (!(exception is OperationCanceledException))
{
Report(exception, s_nonFatalHandler);
}

return false;
}

/// <summary>
/// Report a non-fatal error like <see cref="ReportWithoutCrash"/> but propagates the exception, unless the operation has been cancelled.
/// </summary>
/// <returns>False to propagate the exception.</returns>
[DebuggerHidden]
public static bool ReportWithoutCrashUnlessCanceledAndPropagate(Exception exception, CancellationToken cancellationToken)
{
if (!IsCurrentOperationBeingCancelled(exception, cancellationToken))
{
Report(exception, s_nonFatalHandler);
}

return false;
}

private static readonly object s_reportedMarker = new();

private static void Report(Exception exception, Action<Exception>? handler)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ void errorHandlingBody(int i)
{
body(i);
}
catch (Exception e) when (FatalError.ReportUnlessCanceled(e, cancellationToken))
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e, cancellationToken))
{
throw ExceptionUtilities.Unreachable;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/Core/Portable/Operations/ControlFlowGraph.cs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ internal static ControlFlowGraph CreateCore(IOperation operation, string argumen
Debug.Assert(controlFlowGraph.OriginalOperation == operation);
return controlFlowGraph;
}
catch (Exception e) when (FatalError.ReportWithoutCrashUnlessCanceled(e))
catch (Exception e) when (FatalError.ReportAndCatchUnlessCanceled(e))
{
// Log a Non-fatal-watson and then ignore the crash in the attempt of getting flow graph.
Debug.Assert(false, "\n" + e.ToString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
Sub()
Try
VisitModule(m)
Catch e As Exception When FatalError.ReportUnlessCanceled(e)
Catch e As Exception When FatalError.ReportAndPropagateUnlessCanceled(e)
Throw ExceptionUtilities.Unreachable
End Try
End Sub),
Expand Down Expand Up @@ -156,7 +156,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
Sub()
Try
Visit(m)
Catch e As Exception When FatalError.ReportUnlessCanceled(e)
Catch e As Exception When FatalError.ReportAndPropagateUnlessCanceled(e)
Throw ExceptionUtilities.Unreachable
End Try
End Sub),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
Sub()
Try
CompileNamespace(symbol)
Catch e As Exception When FatalError.ReportUnlessCanceled(e)
Catch e As Exception When FatalError.ReportAndPropagateUnlessCanceled(e)
Throw ExceptionUtilities.Unreachable
End Try
End Sub),
Expand Down Expand Up @@ -505,7 +505,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
Sub()
Try
CompileNamedType(symbol, filter)
Catch e As Exception When FatalError.ReportUnlessCanceled(e)
Catch e As Exception When FatalError.ReportAndPropagateUnlessCanceled(e)
Throw ExceptionUtilities.Unreachable
End Try
End Sub),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols.Metadata.PE

Try
Return assembly.LookupTopLevelMetadataType(emittedName, digThroughForwardedTypes:=True)
Catch e As Exception When FatalError.Report(e) ' Trying to get more useful Watson dumps.
Catch e As Exception When FatalError.ReportAndPropagate(e) ' Trying to get more useful Watson dumps.
Throw ExceptionUtilities.Unreachable
End Try
End Function
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
Sub()
Try
visitor(symbol)
Catch e As Exception When FatalError.ReportUnlessCanceled(e)
Catch e As Exception When FatalError.ReportAndPropagateUnlessCanceled(e)
Throw ExceptionUtilities.Unreachable
End Try
End Sub),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public async Task<Document> AddSourceToAsync(Document document, Compilation symb
var fullAssemblyName = symbol.ContainingAssembly.Identity.GetDisplayName();
GlobalAssemblyCache.Instance.ResolvePartialName(fullAssemblyName, out assemblyLocation, preferredCulture: CultureInfo.CurrentCulture);
}
catch (Exception e) when (FatalError.ReportWithoutCrash(e))
catch (Exception e) when (FatalError.ReportAndCatch(e))
{
}
}
Expand Down
Loading

0 comments on commit 9f82a08

Please sign in to comment.