From f7b4b9d6b8bcf7eb4c2798e325480e83a089da4b Mon Sep 17 00:00:00 2001 From: Jason Malinowski Date: Thu, 21 Feb 2019 16:30:24 -0800 Subject: [PATCH] Add telemetry to EventMap.Registry.Invoke to diagnose a crash We're seeing crashes where it seems a workspace is opening a document twice with no intervening close. One theory we have is something is throwing OperationCanceledExceptions as we're notifying events, and as a result we're not notifying everything. We've audited the possible DocumentClose event handlers that would seem to be blamed but we're unable to come up with any proof there is any problems there. --- .../Core/Portable/Utilities/EventMap.cs | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/Workspaces/Core/Portable/Utilities/EventMap.cs b/src/Workspaces/Core/Portable/Utilities/EventMap.cs index 99be564bd62..2f525881f78 100644 --- a/src/Workspaces/Core/Portable/Utilities/EventMap.cs +++ b/src/Workspaces/Core/Portable/Utilities/EventMap.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; +using Microsoft.CodeAnalysis.ErrorReporting; namespace Roslyn.Utilities { @@ -165,13 +166,26 @@ public bool HasHandlers public void RaiseEvent(Action invoker) { - if (this.HasHandlers) + // The try/catch here is to find additional telemetry for https://devdiv.visualstudio.com/DevDiv/_queries/query/71ee8553-7220-4b2a-98cf-20edab701fd1/. + // We've realized there's a problem with our eventing, where if an exception is encountered while calling into subscribers to Workspace events, + // we won't notify all of the callers. The expectation is such an exception would be thrown to the SafeStartNew in the workspace's event queue that + // will raise that as a fatal exception, but OperationCancelledExceptions might be able to propagate through and fault the task we are using in the + // chain. I'm choosing to use ReportWithoutCrashAndPropagate, because if our theory here is correct, it seems the first exception isn't actually + // causing crashes, and so if it turns out this is a very common situation I don't want to make a often-benign situation fatal. + try { - foreach (var registry in _registries) + if (this.HasHandlers) { - registry.Invoke(invoker); + foreach (var registry in _registries) + { + registry.Invoke(invoker); + } } } + catch (Exception e) when (FatalError.ReportWithoutCrashAndPropagate(e)) + { + throw ExceptionUtilities.Unreachable; + } } } } -- GitLab