[2019-12] [corlib] Capture the ExceptionDispatchInfo when rethrowing from TaskContinuation (#19208)mono-6.10.0.95

* [corlib] Capture the ExceptionDispatchInfo when rethrowing from TaskContinuation

In `task.GetAwaiter().OnCompleted(action)` if `action` throws, the
`TaskContinuation.RunOrScheduleAction method will catch the exception and then
call `RuntimeAugments.ReportUnhandledException`.  We must not simply throw the
exception again - in that case we will lose all the frames leading up to the
`catch` in `RunOrScheduleAction`.  Instead use ExceptionDispatchInfo to
caputure the original throw.

Addresses https://github.com/mono/mono/issues/19166

* [test] Add regression test

For annoying reasons we can't run this as a simple NUnit test because the
exception is unhandled on some arbitrary threadpool worker thread, and those
don't show up within an NUnit setup.

Additionally because the unhandled exception in the root domain would normally
terminate the application, we need to run this in a new appdomain.

Co-authored-by: Aleksey Kliger <alklig@microsoft.com>

3 files changed, 165 insertions, 2 deletions

diff --git a/mcs/class/corlib/corert/RuntimeAugments.cs b/mcs/class/corlib/corert/RuntimeAugments.cs
index c1dc3f3cb93..b55c5ea9a4c 100644
--- a/mcs/class/corlib/corert/RuntimeAugments.cs
+++ b/mcs/class/corlib/corert/RuntimeAugments.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Reflection;
+using System.Runtime.ExceptionServices;
 
 namespace Internal.Runtime.Augments {
 	partial class RuntimeAugments {
@@ -7,7 +8,8 @@ namespace Internal.Runtime.Augments {
 
 		public static void ReportUnhandledException (Exception exception)
 		{
-			throw exception;
+			var edi = ExceptionDispatchInfo.Capture (exception);
+			edi.Throw ();
 		}
 
 		internal static ReflectionExecutionDomainCallbacks Callbacks => s_reflectionExecutionDomainCallbacks;
@@ -19,4 +21,4 @@ namespace Internal.Runtime.Augments {
 			return new MissingMetadataException ();
 		}
 	}
-}
\ No newline at end of file
+}
diff --git a/mono/tests/Makefile.am b/mono/tests/Makefile.am
index e37ae991fe7..323ca18627f 100755
--- a/mono/tests/Makefile.am
+++ b/mono/tests/Makefile.am
@@ -689,6 +689,7 @@ TESTS_CS_SRC=		\
 	monitor.cs	\
 	generic-xdomain.2.cs	\
 	threadpool-exceptions7.cs	\
+	threadpool-exceptions8.cs \
 	cross-domain.cs	\
 	generic-unloading.2.cs	\
 	namedmutex-destroy-race.cs	\
@@ -1386,6 +1387,7 @@ PROFILE_DISABLED_TESTS += \
 	monitor.exe	\
 	generic-xdomain.2.exe	\
 	threadpool-exceptions7.exe	\
+	threadpool-exceptions8.exe	\
 	cross-domain.exe	\
 	generic-unloading.2.exe	\
 	appdomain-threadpool-unload.exe
@@ -1599,6 +1601,7 @@ PROFILE_DISABLED_TESTS += \
 	suspend-stress-test.exe \
 	thread6.exe \
 	threadpool-exceptions7.exe \
+	threadpool-exceptions8.exe	\
 	unhandled-exception-7.exe \
 	unhandled-exception-test-case.2.exe \
 	unload-appdomain-on-shutdown.exe \
diff --git a/mono/tests/threadpool-exceptions8.cs b/mono/tests/threadpool-exceptions8.cs
new file mode 100644
index 00000000000..2049b6ed001
--- /dev/null
+++ b/mono/tests/threadpool-exceptions8.cs
@@ -0,0 +1,158 @@
+using System;
+using System.Diagnostics;
+using System.Reflection;
+using System.Runtime.CompilerServices;
+using System.Runtime.ExceptionServices;
+using System.Threading;
+using System.Threading.Tasks;
+
+public class Tests {
+
+	public static int Main (string[] args) {
+		runner ();
+		return 63; // should not be reached
+	}
+
+	public static void runner () {
+		// need to run the test in a domain so that we can deal with unhandled exceptions
+		var ad = AppDomain.CreateDomain ("Inner Domain");
+		var helperType = typeof(TaskAwaiterOnCompletedHelper);
+		var helper = (TaskAwaiterOnCompletedHelper)ad.CreateInstanceAndUnwrap (helperType.Assembly.ToString(), helperType.FullName);
+		var holder = new ResultHolder ();
+		helper.TheTest (holder);
+		// HACK: If everything went well, a thread is running in the other domain and is blocked in OnUnhandled
+		// waiting for AllDone().  Don't send it.  Instead just exit without waiting.  If we send AllDone, the
+		// process will terminate with a 255.  Don't try to unload the domain either, since the other thread
+		// will never finish.
+		// 
+		//helper.AllDone();
+		//AppDomain.Unload (ad);
+		Environment.Exit (holder.Result);
+	}
+
+	public class ResultHolder : MarshalByRefObject  {
+		public ResultHolder () { }
+
+		public int Result { get; set; }
+	}
+
+	public class TaskAwaiterOnCompletedHelper : MarshalByRefObject {
+			
+		public class SpecialExn : Exception {
+			public SpecialExn () : base () {}
+		}
+			
+		public void TheTest (ResultHolder holder)
+		{
+			this.holder = holder;
+			holder.Result = TheRealTest ();
+		}
+
+		ResultHolder holder;
+		
+		public int TheRealTest ()
+		{
+			// Regression test for https://github.com/mono/mono/issues/19166
+			//
+			// Check that if in a call to
+			// t.GetAwaiter().OnCompleted(cb) the callback cb
+			// throws, that the exception's stack trace includes
+			// the method that threw and not just the task
+			// machinery's frames.
+
+			// Calling "WhenCompleted" will throw "SpecialExn"
+			//
+			// If "OnUhandled" is installed as an unhandled exception handler, it will
+			//  capture the stack trace of the SpecialExn and allow WaitForExn() to finish waiting.
+			// The stack trace is expected to include ThrowerMethodInfo
+
+			var helper = this;
+			var d = new UnhandledExceptionEventHandler (helper.OnUnhandled);
+			AppDomain.CurrentDomain.UnhandledException += d;
+
+			// this is TaskToApm.Begin (..., callback) where the callback is helper.WhenCompleted
+			Task.Delay (100).GetAwaiter().OnCompleted (helper.WhenCompleted);
+
+			var wasSet = helper.WaitForExn (10000); // wait upto 10 seconds for the task to throw
+
+			AppDomain.CurrentDomain.UnhandledException -= d;
+
+			if (!wasSet) {
+				Console.WriteLine ("event not set, no exception thrown?");
+				return 1;
+			}
+
+			return 0;
+
+		}
+
+		private ManualResetEventSlim coord;
+		private ManualResetEventSlim coord2;
+
+		private StackFrame[] frames;
+			
+		public TaskAwaiterOnCompletedHelper ()
+		{
+			coord = new ManualResetEventSlim ();
+			coord2 = new ManualResetEventSlim ();
+		}
+
+		public MethodBase ThrowerMethodInfo => typeof(TaskAwaiterOnCompletedHelper).GetMethod (nameof (WhenCompletedThrower));
+
+		[MethodImpl (MethodImplOptions.NoInlining)]
+		public void WhenCompleted ()
+		{
+			WhenCompletedThrower ();
+		}
+
+		[MethodImpl (MethodImplOptions.NoInlining)]
+		public void WhenCompletedThrower ()
+		{
+			throw new SpecialExn ();
+		}
+
+		public void OnUnhandled (object sender, UnhandledExceptionEventArgs args)
+		{
+			if (args.ExceptionObject is SpecialExn exn) {
+				try {
+					var trace = new StackTrace (exn);
+					frames = trace.GetFrames ();
+					if (frames == null) {
+						holder.Result = 2;
+						return;
+					}
+					Console.WriteLine ("got {0} frames ", frames.Length);
+					bool found = false;
+					foreach (var frame in frames) {
+						if (frame.GetMethod ().Equals (ThrowerMethodInfo)) {
+							found = true;
+							break;
+						}
+					}
+					if (!found) {
+						Console.WriteLine ("expected to see {0} in stack trace, but it wasn't there", ThrowerMethodInfo.ToString());
+						holder.Result = 3;
+						return;
+					}
+				} finally {
+					coord.Set ();
+				
+					coord2.Wait ();
+				}
+			}
+		}
+
+		public StackFrame[] CapturedStackTraceFrames => frames;
+
+
+		public bool WaitForExn (int timeoutMilliseconds)
+		{
+			return coord.Wait (timeoutMilliseconds);
+		}
+
+		public void AllDone ()
+		{
+			coord2.Set ();
+		}
+	}
+}