From 67770358f6eacd52b3cfdfd30c2b7f4e2d587c92 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Wed, 5 May 2021 09:44:34 -0600 Subject: [PATCH] Crash the process on unexpected failures when creating a `JoinableTask` Strictly speaking this is not necessary, as an exception would be thrown to the caller and no lingering adverse effects to the state of `JoinableTaskContext` are left behind. But we find this change useful because this is scheduling code and should be very reliable. Recently an `OutOfMemoryException` was thrown from this code (in a process with plenty of memory) and led other code to hang because it didn't expect JTF itself to fail. See #843 for more info on that. --- .../JoinableTaskFactory.cs | 38 +++++++++++++------ 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.VisualStudio.Threading/JoinableTaskFactory.cs b/src/Microsoft.VisualStudio.Threading/JoinableTaskFactory.cs index 1c6377337..b54aa25a8 100644 --- a/src/Microsoft.VisualStudio.Threading/JoinableTaskFactory.cs +++ b/src/Microsoft.VisualStudio.Threading/JoinableTaskFactory.cs @@ -689,21 +689,37 @@ private JoinableTask RunAsync(Func> asyncMethod, bool synchronousl [SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "All exceptions are forwarded to the caller by another means.")] private void ExecuteJob(Func asyncMethod, JoinableTask job) { - using (var framework = new RunFramework(this, job)) + try { - Task asyncMethodResult; - try + using (var framework = new RunFramework(this, job)) { - asyncMethodResult = asyncMethod(); - } - catch (Exception ex) - { - var tcs = new TaskCompletionSource(); - tcs.SetException(ex); - asyncMethodResult = tcs.Task; + Task asyncMethodResult; + try + { + asyncMethodResult = asyncMethod(); + } + catch (Exception ex) + { + var tcs = new TaskCompletionSource(); + tcs.SetException(ex); + asyncMethodResult = tcs.Task; + } + + job.SetWrappedTask(asyncMethodResult); } + } + catch (Exception ex) when (FailFast(ex)) + { + // We use a crashing exception filter to capture all the detail possible (even before unwinding the callstack) + // when an exception is thrown from this critical method. + // In particular, we have seen the WeakReference object that is instantiated by "new RunFramework" throw OutOfMemoryException. + throw Assumes.NotReachable(); + } - job.SetWrappedTask(asyncMethodResult); + static bool FailFast(Exception ex) + { + Environment.FailFast("Unexpected exception thrown in critical scheduling code.", ex); + throw Assumes.NotReachable(); } }