[WIP] Don't try to access if thread is not running #3174

forki · 2017-06-02T10:00:44Z

forki · 2017-06-02T16:03:36Z

I think @chillitom is right and there might be races here. but it already improve things...

KevinRansom · 2017-06-02T20:39:30Z

vsintegration/src/FSharp.Editor/Common/RoslynHelpers.fs

+                                                            tcs.TrySetCanceled(cancellationToken)  |> ignore
+                                                | exn ->
+                                                    disposeReg()
+                                                    if tcs.Task.Status = TaskStatus.Running then


I can see cancelling the task when it is not still running is a problem. But shouldn't we call set exception whether the task is running or not?

Thema actually issue issue issue setexception. I only added these other cases for symmetry

KevinRansom · 2017-06-03T07:17:45Z

@dsyme

Can you take a look at this please. The code is on the face of it all right, it's just I can't help thinking that it is treating the symptoms rather than the issue. I wonder if there is something wrong with the implementation.

Getting an ExecutionEngineException out of a normal use of an API seems a bit excessive.

forki · 2017-06-03T13:27:31Z

Yes it issue definetly only symptomatic

dsyme · 2017-06-05T14:46:22Z

@KevinRansom @forki This looks like a sound fix to me - not just symptomatic. My understanding is that the cancellation registration has triggered, putting the task in "cancelled" state, just prior to the exception continuation being executed.

There should be no race condition since only one continuation will be called (and we have already disposed of the cancellation registration at this point)

I agree that calling TrySetException should no cause such a catastrophic error when a task has already been cancelled, it seems quite wrong. Perhaps there is something else lurking there.

KevinRansom · 2017-06-05T17:21:35Z

@stephentoub is taking a look.

stephentoub · 2017-06-05T17:25:19Z

A few things:

This should not be necessary; if the task has already completed, calling TrySet on the tcs is a nop.
Even if it were a problem, the fix then has a race condition, as the task could finish after the status check and before the call.
Nothing in the task implementation throws an ExecutionEngineException. That exception is usually an indication of something going very wrong, e.g. having a bad P/Invoke somewhere, loading a bad assembly, a fail fast, etc.
I unfortunately don't read German and thus can't interpret the error message in the screenshot well. The call stack is also hiding "[External code]", which would hopefully shed more light on where the exception is actually coming from. Can you share the raw text of the exception message so I can run it through a translator, and also unhide the external frames in the call stack?

matthid · 2017-06-05T17:48:23Z

@stephentoub

This should not be necessary; if the task has already completed, calling TrySet on the tcs is a nop.

I think everyone agrees on that, but not the runtime :)

Even if it were a problem, the fix then has a race condition, as the task could finish after the status check and before the call.

The task can only finish via TaskCompletionSource? There are only 2 sources where we touch that:

a) By the CancellationRegistration -> We already disposed that at this place (before doing the check)
b) By another callback -> it should be garantueed that only a single one is called.

We don't leak the TaskCompletionSource handle anywhere as far as I can see.

Nothing in the task implementation throws an ExecutionEngineException. That exception is usually an indication of something going very wrong, e.g. having a bad P/Invoke somewhere, loading a bad assembly, a fail fast, etc.

Yes ultimately it looks like a runtime bug which needs a real fix. But the workaround looks good.

I unfortunately don't read German and thus can't interpret the error message in the screenshot well. The call stack is also hiding "[External code]", which would hopefully shed more light on where the exception is actually coming from. Can you share the raw text of the exception message so I can run it through a translator, and also unhide the external frames in the call stack?

@forki posted a way to reproduce here. Maybe you can try to reproduce? The message only really contains some generic text and some address pointers I don't think it will help.

KevinRansom · 2017-06-05T17:57:08Z

@matthid I will add it to the long list of things I need to look at.

forki · 2017-06-06T06:19:28Z

if the task has already completed, calling TrySet on the tcs is a nop.

@stephentoub that's what I thought as well. But something in TrySetException seems to create that crash.
The change with TrySetCanceled was only for symmetry.

stephentoub · 2017-06-06T13:18:32Z

something in TrySetException seems to create that crash

If something in TrySetException is crashing, most likely it actually has little to do with it, and, for example, something somewhere else has corrupted memory that TrySetException touches (e.g. something gets JIT'd during the call and that experiences a problem). If the Task is already completed, TrySetException just checks a flag and returns false. You can see the code in the reference source.

What is the full stack of the exception, including the hidden "external" frames?

forki · 2017-06-06T13:43:14Z

@stephentoub #3171 (comment) is all I have

stephentoub · 2017-06-06T13:44:56Z

#3171 (comment) is all I have

@forki, are you not able to repro the problem? If you are, can you right-click on the "[Externer Code]" entry in the call stack and select the menu option to include external code?

forki · 2017-06-06T13:47:40Z

I was able to repro before the change yes. But I'm not in work office this week so can't do that. But you should be able to repro with the steps I listed.

chillitom · 2017-06-06T14:44:15Z

I've followed all the steps but now can't get the repro to trigger! Frustrating. @forki do you remember which commit exactly you tested against?

Is there a way to be sure I'm running my custom built version of the F# tools? Checking in Tools and Extensions shows a data from 4 days ago.

chillitom · 2017-06-06T15:08:44Z

Got it. Here's the stack with external code displayed.

stephentoub · 2017-06-06T15:14:40Z

Thanks, @chillitom. That stack shows that:

The Task wasn't actually completed yet. Calling TrySetException on it is completing the task.
Completing the task is causing continuations of the task to execute, and continuations of that task, etc.
Eventually it ends up running code from Roslyn that issues a FailFast, hence the ExecutionEngineException.

chillitom · 2017-06-06T15:15:13Z

 	Microsoft.CodeAnalysis.Workspaces.dll!Microsoft.CodeAnalysis.ErrorReporting.FatalError.Report(System.Exception exception, System.Action<System.Exception> handler)	Unknown
 	Microsoft.CodeAnalysis.Workspaces.dll!Roslyn.Utilities.TaskExtensions.ReportFatalErrorWorker(System.Threading.Tasks.Task task, object continuationFunction)	Unknown
 	mscorlib.dll!System.Threading.Tasks.ContinuationTaskFromTask.InnerInvoke()	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.Execute()	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.ExecutionContextCallback(object obj)	Unknown
 	mscorlib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)	Unknown
 	mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.ExecuteWithThreadLocal(ref System.Threading.Tasks.Task currentTaskSlot)	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.ExecuteEntry(bool bPreventDoubleExecution)	Unknown
 	mscorlib.dll!System.Threading.Tasks.ThreadPoolTaskScheduler.TryExecuteTaskInline(System.Threading.Tasks.Task task, bool taskWasPreviouslyQueued)	Unknown
 	mscorlib.dll!System.Threading.Tasks.TaskScheduler.TryRunInline(System.Threading.Tasks.Task task, bool taskWasPreviouslyQueued)	Unknown
 	mscorlib.dll!System.Threading.Tasks.TaskContinuation.InlineIfPossibleOrElseQueue(System.Threading.Tasks.Task task, bool needsProtection)	Unknown
 	mscorlib.dll!System.Threading.Tasks.StandardTaskContinuation.Run(System.Threading.Tasks.Task completedTask, bool bCanInlineContinuationTask)	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.FinishContinuations()	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.FinishStageThree()	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.FinishStageTwo()	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.Finish(bool bUserDelegateExecuted)	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task<System.Threading.Tasks.TaskExtensions.VoidResult>.TrySetException(object exceptionObject)	Unknown
 	mscorlib.dll!System.Threading.Tasks.UnwrapPromise<System.Threading.Tasks.TaskExtensions.VoidResult>.TrySetFromTask(System.Threading.Tasks.Task task, bool lookForOce)	Unknown
 	mscorlib.dll!System.Threading.Tasks.UnwrapPromise<System.Threading.Tasks.TaskExtensions.VoidResult>.InvokeCore(System.Threading.Tasks.Task completingTask)	Unknown
 	mscorlib.dll!System.Threading.Tasks.UnwrapPromise<System.Threading.Tasks.TaskExtensions.VoidResult>.Invoke(System.Threading.Tasks.Task completingTask)	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.FinishContinuations()	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.FinishStageThree()	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.FinishStageTwo()	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.Finish(bool bUserDelegateExecuted)	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task<System.Threading.Tasks.VoidTaskResult>.TrySetException(object exceptionObject)	Unknown
 	mscorlib.dll!System.Runtime.CompilerServices.AsyncTaskMethodBuilder<System.Threading.Tasks.VoidTaskResult>.SetException(System.Exception exception)	Unknown
 	Microsoft.CodeAnalysis.EditorFeatures.dll!Microsoft.CodeAnalysis.Editor.Tagging.AbstractAsynchronousTaggerProvider<Microsoft.CodeAnalysis.Editor.Shared.Tagging.NavigableHighlightTag>.TagSource.RecomputeTagsAsync(object oldState, Microsoft.VisualStudio.Text.SnapshotPoint? caretPosition, Microsoft.CodeAnalysis.Text.TextChangeRange? textChangeRange, System.Collections.Generic.List<Microsoft.CodeAnalysis.Editor.DocumentSnapshotSpan> spansToTag, System.Collections.Immutable.ImmutableDictionary<Microsoft.VisualStudio.Text.ITextBuffer, Microsoft.CodeAnalysis.Editor.Shared.Tagging.TagSpanIntervalTree<Microsoft.CodeAnalysis.Editor.Shared.Tagging.NavigableHighlightTag>> oldTagTrees, System.Threading.CancellationToken cancellationToken)	Unknown
 	mscorlib.dll!System.Runtime.CompilerServices.AsyncMethodBuilderCore.MoveNextRunner.InvokeMoveNext(object stateMachine)	Unknown
 	mscorlib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)	Unknown
 	mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)	Unknown
 	mscorlib.dll!System.Runtime.CompilerServices.AsyncMethodBuilderCore.MoveNextRunner.Run()	Unknown
 	mscorlib.dll!System.Threading.Tasks.AwaitTaskContinuation.RunOrScheduleAction(System.Action action, bool allowInlining, ref System.Threading.Tasks.Task currentTask)	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.FinishContinuations()	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.FinishStageThree()	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.FinishStageTwo()	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.Finish(bool bUserDelegateExecuted)	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task<System.Threading.Tasks.VoidTaskResult>.TrySetException(object exceptionObject)	Unknown
 	mscorlib.dll!System.Runtime.CompilerServices.AsyncTaskMethodBuilder<System.Threading.Tasks.VoidTaskResult>.SetException(System.Exception exception)	Unknown
 	Microsoft.CodeAnalysis.EditorFeatures.dll!Microsoft.CodeAnalysis.Editor.Implementation.ReferenceHighlighting.ReferenceHighlightingViewTaggerProvider.ProduceTagsAsync(Microsoft.CodeAnalysis.Editor.Tagging.TaggerContext<Microsoft.CodeAnalysis.Editor.Shared.Tagging.NavigableHighlightTag> context, Microsoft.VisualStudio.Text.SnapshotPoint position, Microsoft.CodeAnalysis.Workspace workspace, Microsoft.CodeAnalysis.Document document)	Unknown
 	mscorlib.dll!System.Runtime.CompilerServices.AsyncMethodBuilderCore.MoveNextRunner.InvokeMoveNext(object stateMachine)	Unknown
 	mscorlib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)	Unknown
 	mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)	Unknown
 	mscorlib.dll!System.Runtime.CompilerServices.AsyncMethodBuilderCore.MoveNextRunner.Run()	Unknown
 	mscorlib.dll!System.Threading.Tasks.AwaitTaskContinuation.RunOrScheduleAction(System.Action action, bool allowInlining, ref System.Threading.Tasks.Task currentTask)	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.FinishContinuations()	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.FinishStageThree()	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.FinishStageTwo()	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task.Finish(bool bUserDelegateExecuted)	Unknown
 	mscorlib.dll!System.Threading.Tasks.Task<System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Editor.DocumentHighlights>>.TrySetException(object exceptionObject)	Unknown
 	mscorlib.dll!System.Threading.Tasks.TaskCompletionSource<System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Editor.DocumentHighlights>>.TrySetException(System.Exception exception)	Unknown
>	FSharp.Editor.dll!Microsoft.VisualStudio.FSharp.Editor.RoslynHelpers.StartAsyncAsTask@106-3<System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Editor.DocumentHighlights>>.Invoke(System.Exception _arg2) Line 107	F#
 	FSharp.Core.dll!Microsoft.FSharp.Control.CancellationTokenOps.StartWithContinuations@1298-3.Invoke(System.Runtime.ExceptionServices.ExceptionDispatchInfo x)	Unknown
 	FSharp.Core.dll!<StartupCode$FSharp-Core>[email protected](Microsoft.FSharp.Core.FSharpFunc<Microsoft.FSharp.Core.Unit, Microsoft.FSharp.Control.FakeUnitValue> action)	Unknown
 	FSharp.Core.dll!Microsoft.FSharp.Control.Trampoline.ExecuteAction(Microsoft.FSharp.Core.FSharpFunc<Microsoft.FSharp.Core.Unit, Microsoft.FSharp.Control.FakeUnitValue> firstAction)	Unknown
 	FSharp.Core.dll!Microsoft.FSharp.Control.TrampolineHolder.Protect(Microsoft.FSharp.Core.FSharpFunc<Microsoft.FSharp.Core.Unit, Microsoft.FSharp.Control.FakeUnitValue> firstAction)	Unknown
 	FSharp.Core.dll!<StartupCode$FSharp-Core>[email protected](object state)	Unknown
 	mscorlib.dll!System.Threading.QueueUserWorkItemCallback.WaitCallback_Context(object state)	Unknown
 	mscorlib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)	Unknown
 	mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)	Unknown
 	mscorlib.dll!System.Threading.QueueUserWorkItemCallback.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem()	Unknown
 	mscorlib.dll!System.Threading.ThreadPoolWorkQueue.Dispatch()	Unknown
 	mscorlib.dll!System.Threading._ThreadPoolWaitCallback.PerformWaitCallback()	Unknown

Managed Debugging Assistant 'FatalExecutionEngineError'  occurred
  HResult=0x00000000
  Message=**Managed Debugging Assistant 'FatalExecutionEngineError' :** 'The runtime has encountered a fatal error. The address of the error was at 0x1d8ad637, on thread 0x2ce0. The error code is 0x80131623. This error may be a bug in the CLR or in the unsafe or non-verifiable portions of user code. Common sources of this bug include user marshaling errors for COM-interop or PInvoke, which may corrupt the stack.'

forki · 2017-06-06T15:27:05Z

@stephentoub so what would be the proper fix?

stephentoub · 2017-06-06T15:29:41Z

so what would be the proper fix?

I have no idea... it's unrelated to Tasks as far as I can tell (Tasks here are just the infrastructure that's invoking the delegates that are then issuing the fail fast) and likely specific to the VS environment, the project system, Roslyn, etc. The fail fast being generated by VS should contain some details on what the problem was... if it's not getting output to the debugger or the console, you might look in the event log. Others like @KevinRansom will likely have a better idea.

chillitom · 2017-06-06T15:37:52Z

here are the last few lines of debug console output:

devenv.exe Information: 0 : Reactor: <-- DocumentHighlights.ParseAndCheckFileInProject, took 0.0887 ms
devenv.exe Information: 0 : Reactor: enqueue DocumentHighlights.GetSymbolUseAtLocation (C:\Projects\visualfsharp_bug_3171\visualfsharp_bug_3171\Bug3171.fs), length 0
devenv.exe Information: 0 : Reactor: --> DocumentHighlights.GetSymbolUseAtLocation (C:\Projects\visualfsharp_bug_3171\visualfsharp_bug_3171\Bug3171.fs), remaining 0
devenv.exe Information: 0 : Reactor: enqueue GCFinalizer.FSharpCheckFileResults.DecrementUsageCountOnIncrementalBuilder (C:\Projects\visualfsharp_bug_3171\visualfsharp_bug_3171\Bug3171.fs), length 0
Exception thrown: 'Microsoft.FSharp.Compiler.ErrorLogger.ReportedError' in FSharp.Core.dll
devenv.exe Information: 0 : Reactor: <-- DocumentHighlights.GetSymbolUseAtLocation, took 92.9174 ms
devenv.exe Information: 0 : Reactor: --> GCFinalizer.FSharpCheckFileResults.DecrementUsageCountOnIncrementalBuilder (C:\Projects\visualfsharp_bug_3171\visualfsharp_bug_3171\Bug3171.fs), remaining 0
devenv.exe Information: 0 : Reactor: <-- GCFinalizer.FSharpCheckFileResults.DecrementUsageCountOnIncrementalBuilder, took 0.1032 ms
The thread 0x1cac has exited with code 0 (0x0).
The thread 0x1b74 has exited with code 0 (0x0).
devenv.exe Information: 0 : The thread 0x1eb4 has exited with code 0 (0x0).
The thread 0x4310 has exited with code 0 (0x0).
The thread 0x4288 has exited with code 0 (0x0).
The thread 0x4388 has exited with code 0 (0x0).
The thread 0x3c20 has exited with code 0 (0x0).
The thread 0x14ac has exited with code 0 (0x0).```

forki · 2017-06-06T15:53:17Z

please remember:

if tcs.Task.Status = TaskStatus.Running then
    tcs.TrySetException(exn) |> ignore

^^ this is the code that "fixed" it.
So the Status says the task wasn't running and @stephentoub's first assumption is not really valid - at least if you ask tcs.Task.Status

dsyme · 2017-06-06T16:01:58Z

@forki Do you know what tcs.Task.Status actually was in this case? thanks.

forki · 2017-06-06T16:03:09Z

@chillitom can you please check?

stephentoub · 2017-06-06T16:03:36Z

Do you know what tcs.Task.Status actually was in this case? thanks.

In particular, for example, the "fix" changed it to use TrySetCanceled instead of TrySetException. So it may not have been Running, it may have been, for example, WaitingForActivation, in which case TrySetCanceled would have completed the task instead of TrySetException. And maybe the continuations are doing something different based on the completion status of the task, e.g. maybe it's the exception that's triggering the fail fast.

In particular, for example, the "fix" changed it to complete the Task only if it was running, but it may have been, for example, WaitingForActivation, in which case it would have skipped completing the task and the continuations wouldn't have run at all.

forki · 2017-06-06T16:05:53Z

for example, the "fix" changed it to use TrySetCanceled instead of TrySetException

no it does not. It's just a bit differently aligned so that it might look like this if you are not used to F#

stephentoub · 2017-06-06T16:06:30Z

Yes, I misspoke. But regardless, it changed it to complete the task only if it was Running... it may have been another non-completed status, like WaitingForActivation, in which case the change simply didn't complete the task at all, preventing any of its continuations from running, and it's the body of the continuations that are issuing the fail fast.

chillitom · 2017-06-06T16:08:18Z

I'm having trouble inspecting anything, watches, autos and the immediate window all unable to give me any information about task state and getting nothing at all when hovering the source. I have a BP set on disposeReg so it should be before the stack gets unwound by the fatal ex.

forki · 2017-06-06T16:09:38Z

@chillitom there is an easy trick: failwithf "%A" tcs.Task.Status ;-)

chillitom · 2017-06-06T16:21:26Z

WaitingForActivation

stephentoub · 2017-06-06T16:25:12Z

WaitingForActivation

Thanks. Which means the "fix" really just preventing the continuations from running, by never completing the task. That'd be like fixing a bug by simply not calling the method that throws the exception or issues the fail fast. Understanding the right fix requires understanding why that code is throwing / fail fasting.

dsyme · 2017-06-06T16:32:13Z

I must admit I find Roslyn's predilection for using FailFast a bit disturbing, though they look like odd corner cases where it is used. We hit this in one other case (unrelated to this - see #3071)

dsyme · 2017-06-06T16:37:23Z

There are only two calls to ReportFatalError in Roslyn - they are related to whacky corner cases in task settings which I don't understand but they do seem to be plausibly related

Why Roslyn has to turn these into catastrophic fatal errors I don't know.

stephentoub · 2017-06-06T16:44:05Z

they are related to whacky corner cases in task settings which I don't understand but they do seem to be plausibly related

Just took a quick look. Two things:

The fail fast isn't actually related to the corner case. This code is just creating a continuation, and then off of that continuation it's creating another that will fail fast if the first failed with an exception. It'd be like wrapping the supplied continuation delegate in a try/catch where the catch fail fast'd.
While I don't believe it's related, the corner case it's working around is this. You have a Task A and you create a continuation Task B off of it, but you create that Task B with a cancellation token. That means that if Task B's cancellation token is canceled while Task A is running, Task B may complete while Task A is running, even though it's logically a continuation off of A. Then if there's a continuation Task C off of B, because B could complete due to cancellation while A is running, Task C could start running while A is still running, even though it's logically a continuation from B. That's fine, unless C expects that all of its antecedents have completed. So this code is just including the LazyCancellation option, which tells task B "even if you're explicitly canceled, don't complete until your antecedent has completed, too".

dsyme · 2017-06-06T16:48:24Z

@stephentoub Thanks for the explanation. I think I'll need to try to repro this.

While I don't believe it's related, the corner case it's working around is this....

I have to admit that explanation makes me more and more convinced that the F# async design philosophy of "compose task generators, not tasks" is fundamentally the right one :)

dsyme · 2017-06-06T16:49:56Z

@forki I've put "WIP" on this since the current fix definitely isn't right (as @stephentoub mentions it is just suppressing the completion of the task in some cases)

dsyme · 2017-06-06T17:18:58Z

Well, I can repro where the first exception is being raised. However I still need to understand why this has become more catastrophic after the change to how we start tasks

Here's a related stack which shows that the F# compiler routine GetSymbolUseAtLocation is raising an exception, and that this is causing an F# async related to the Microsoft.CodeAnalysis.Editor.Implementation.ReferenceHighlighting.ReferenceHighlightingViewTagger to end as an exception. The technique we are using to pass this exception on to Roslyn must be wrong - or else we were previously swallowing the exception.

" at Microsoft.FSharp.Core.Operators.Raise[T](Exception exn) in C:\GitHub\dsyme\visualfsharp\src\fsharp\FSharp.Core\prim-types.fs:line 3803\r\n at Microsoft.FSharp.Compiler.Import.ImportTypeRefData(ImportMap env, range m, ILScopeRef scoref, String[] path, String typeName) in C:\GitHub\dsyme\visualfsharp\src\fsharp\import.fs:line 96\r\n at Microsoft.FSharp.Compiler.Import.ImportILTypeRef(ImportMap env, range m, ILTypeRef tref) in C:\GitHub\dsyme\visualfsharp\src\fsharp\import.fs:line 141\r\n at Microsoft.FSharp.Compiler.Import.ImportILType(ImportMap env, range m, FSharpList1 tinst, ILType typ) in C:\\GitHub\\dsyme\\visualfsharp\\src\\fsharp\\import.fs:line 170\r\n at Microsoft.FSharp.Compiler.Infos.ImportILType(ILScopeRef scoref, ImportMap amap, range m, FSharpList1 importInst, ILType ilty) in C:\GitHub\dsyme\visualfsharp\src\fsharp\infos.fs:line 41\r\n at [email protected](ILType ity) in C:\GitHub\dsyme\visualfsharp\src\fsharp\infos.fs:line 139\r\n at Microsoft.FSharp.Primitives.Basics.List.choose[T,TResult](FSharpFunc2 f, FSharpList1 xs) in C:\GitHub\dsyme\visualfsharp\src\fsharp\FSharp.Core\local.fs:line 195\r\n at Microsoft.FSharp.Collections.ListModule.Choose[T,TResult](FSharpFunc2 chooser, FSharpList1 list) in C:\GitHub\dsyme\visualfsharp\src\fsharp\FSharp.Core\list.fs:line 155\r\n at Microsoft.FSharp.Compiler.Infos.GetImmediateInterfacesOfType(SkipUnrefInterfaces skipUnref, TcGlobals g, ImportMap amap, range m, TType typ) in C:\GitHub\dsyme\visualfsharp\src\fsharp\infos.fs:line 137\r\n at [email protected](Int32 ndeep, TType typ, Tuple3 tupledArg) in C:\\GitHub\\dsyme\\visualfsharp\\src\\fsharp\\infos.fs:line 183\r\n at Microsoft.FSharp.Compiler.Infos.FoldHierarchyOfTypeAux[a](Boolean followInterfaces, AllowMultiIntfInstantiations allowMultiIntfInst, SkipUnrefInterfaces skipUnref, FSharpFunc2 visitor, TcGlobals g, ImportMap amap, range m, TType typ, a acc) in C:\GitHub\dsyme\visualfsharp\src\fsharp\infos.fs:line 227\r\n at Microsoft.FSharp.Compiler.AbstractIL.Internal.Library.Apply@999.Invoke(Unit unitVar0) in C:\GitHub\dsyme\visualfsharp\src\absil\illib.fs:line 1004\r\n at Microsoft.FSharp.Compiler.AbstractIL.Internal.Library.MemoizationTable2.Apply(T x) in C:\\GitHub\\dsyme\\visualfsharp\\src\\absil\\illib.fs:line 999\r\n at Microsoft.FSharp.Compiler.NameResolution.ResolveCompletionsInType(NameResolver ncenv, NameResolutionEnv nenv, ResolveCompletionTargets completionTargets, range m, AccessorDomain ad, Boolean statics, TType typ) in C:\\GitHub\\dsyme\\visualfsharp\\src\\fsharp\\NameResolution.fs:line 3406\r\n at Microsoft.FSharp.Compiler.NameResolution.ResolvePartialLongIdentInType(NameResolver ncenv, NameResolutionEnv nenv, ResolveCompletionTargets isApplicableMeth, range m, AccessorDomain ad, Boolean statics, FSharpList1 plid, TType typ) in C:\GitHub\dsyme\visualfsharp\src\fsharp\NameResolution.fs:line 3606\r\n at Microsoft.FSharp.Compiler.NameResolution.ResolvePartialLongIdentPrim(NameResolver ncenv, NameResolutionEnv nenv, ResolveCompletionTargets isApplicableMeth, FullyQualifiedFlag fullyQualified, range m, AccessorDomain ad, FSharpList1 plid, Boolean allowObsolete) in C:\\GitHub\\dsyme\\visualfsharp\\src\\fsharp\\NameResolution.fs:line 3881\r\n at Microsoft.FSharp.Compiler.SourceCodeServices.TypeCheckInfo.GetEnvironmentLookupResolutions(NameResolutionEnv nenv, AccessorDomain ad, range m, FSharpList1 plid, TypeNameResolutionFlag filterCtors, Boolean showObsolete) in C:\GitHub\dsyme\visualfsharp\src\fsharp\vs\service.fs:line 475\r\n at Microsoft.FSharp.Compiler.SourceCodeServices.TypeCheckInfo.GetDeclaredItems[a](FSharpOption1 parseResultsOpt, String lineStr, FSharpOption1 origLongIdentOpt, Int32 colAtEndOfNamesAndResidue, FSharpOption1 residueOpt, Int32 line, Int32 loc, TypeNameResolutionFlag filterCtors, ResolveOverloads resolveOverloads, FSharpFunc2 hasTextChangedSinceLastTypecheck, Boolean isInRangeOperator, FSharpFunc2 allSymbols) in C:\\GitHub\\dsyme\\visualfsharp\\src\\fsharp\\vs\\service.fs:line 713\r\n at Microsoft.FSharp.Compiler.SourceCodeServices.TypeCheckInfo.GetDeclItemsForNamesAtPosition(CompilationThreadToken ctok, FSharpOption1 parseResultsOpt, FSharpOption1 origLongIdentOpt, FSharpOption1 residueOpt, Int32 line, String lineStr, Int32 colAtEndOfNamesAndResidue, TypeNameResolutionFlag filterCtors, ResolveOverloads resolveOverloads, FSharpFunc2 getAllSymbols, FSharpFunc2 hasTextChangedSinceLastTypecheck) in C:\GitHub\dsyme\visualfsharp\src\fsharp\vs\service.fs:line 876\r\n at Microsoft.FSharp.Compiler.SourceCodeServices.TypeCheckInfo.GetSymbolUseAtLocation(CompilationThreadToken ctok, Int32 line, String lineStr, Int32 colAtEndOfNames, FSharpList1 names) in C:\\GitHub\\dsyme\\visualfsharp\\src\\fsharp\\vs\\service.fs:line 1126\r\n at <StartupCode$FSharp-Compiler-Private>[email protected](CompilationThreadToken ctok, TypeCheckInfo scope) in C:\\GitHub\\dsyme\\visualfsharp\\src\\fsharp\\vs\\service.fs:line 1827\r\n at <StartupCode$FSharp-Compiler-Private>[email protected](CompilationThreadToken ctok) in C:\\GitHub\\dsyme\\visualfsharp\\src\\fsharp\\vs\\service.fs:line 1760\r\n at <StartupCode$FSharp-Compiler-Private>[email protected](CompilationThreadToken ctok) in C:\\GitHub\\dsyme\\visualfsharp\\src\\fsharp\\vs\\Reactor.fs:line 156\r\n at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)\r\n at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\r\n at Microsoft.CodeAnalysis.Editor.Implementation.ReferenceHighlighting.ReferenceHighlightingViewTaggerProvider.<ProduceTagsAsync>d__12.MoveNext()\r\n at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)\r\n at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)\r\n at Microsoft.CodeAnalysis.Editor.Tagging.AbstractAsynchronousTaggerProvider1.TagSource.d__83.MoveNext()"

dsyme · 2017-06-06T17:22:21Z

Ugh, previously we just returned Unchecked.defaultof<_> as the task result if an exception occurred, just swallowing all exceptions and never giving them to Roslyn. My change passed the exceptions to Roslyn and Roslyn Sure Don't Like Exceptions.

I'll submit a PR to simply do what we used to do for exceptions.

KevinRansom · 2017-06-06T17:24:16Z

@dsyme Thanks mate ...

dsyme · 2017-06-06T17:32:29Z

Closing in favour of #3182

That fix still needs to be verified however

msftclas added the cla-already-signed label Jun 2, 2017

Don't try to access if thread is not running - fixes dotnet#3171

fcbb1ce

forki force-pushed the i3171 branch from b1c207a to fcbb1ce Compare June 2, 2017 10:01

forki mentioned this pull request Jun 2, 2017

Compiler error causes VS2017 to crash #3171

Closed

KevinRansom reviewed Jun 2, 2017

View reviewed changes

dsyme approved these changes Jun 5, 2017

View reviewed changes

dsyme changed the title ~~Don't try to access if thread is not running~~ [WIP] Don't try to access if thread is not running Jun 6, 2017

dsyme closed this Jun 6, 2017

[WIP] Don't try to access if thread is not running #3174

[WIP] Don't try to access if thread is not running #3174

Conversation

forki commented Jun 2, 2017

forki commented Jun 2, 2017

KevinRansom Jun 2, 2017

Choose a reason for hiding this comment

forki Jun 2, 2017 • edited by dsyme Loading

Choose a reason for hiding this comment

KevinRansom commented Jun 3, 2017

forki commented Jun 3, 2017 via email • edited by dsyme Loading

dsyme commented Jun 5, 2017 • edited Loading

KevinRansom commented Jun 5, 2017

stephentoub commented Jun 5, 2017

matthid commented Jun 5, 2017

KevinRansom commented Jun 5, 2017

forki commented Jun 6, 2017

stephentoub commented Jun 6, 2017 • edited Loading

forki commented Jun 6, 2017

stephentoub commented Jun 6, 2017

forki commented Jun 6, 2017

chillitom commented Jun 6, 2017 • edited Loading

chillitom commented Jun 6, 2017

stephentoub commented Jun 6, 2017

chillitom commented Jun 6, 2017

forki commented Jun 6, 2017

stephentoub commented Jun 6, 2017

chillitom commented Jun 6, 2017

forki commented Jun 6, 2017

dsyme commented Jun 6, 2017

forki commented Jun 6, 2017

stephentoub commented Jun 6, 2017 • edited Loading

forki commented Jun 6, 2017

stephentoub commented Jun 6, 2017 • edited Loading

chillitom commented Jun 6, 2017

forki commented Jun 6, 2017

chillitom commented Jun 6, 2017

stephentoub commented Jun 6, 2017

dsyme commented Jun 6, 2017

dsyme commented Jun 6, 2017 • edited Loading

stephentoub commented Jun 6, 2017

dsyme commented Jun 6, 2017

dsyme commented Jun 6, 2017

dsyme commented Jun 6, 2017

dsyme commented Jun 6, 2017

KevinRansom commented Jun 6, 2017

dsyme commented Jun 6, 2017

forki Jun 2, 2017 •

edited by dsyme

Loading

forki commented Jun 3, 2017 via email •

edited by dsyme

Loading

dsyme commented Jun 5, 2017 •

edited

Loading

stephentoub commented Jun 6, 2017 •

edited

Loading

chillitom commented Jun 6, 2017 •

edited

Loading

stephentoub commented Jun 6, 2017 •

edited

Loading

stephentoub commented Jun 6, 2017 •

edited

Loading

dsyme commented Jun 6, 2017 •

edited

Loading