[Xamarin.Android.Build.Tasks] IOException & Console.InputEncoding (#4735)

Fixes: https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1130414

Context: 0d6fd83

For some unknown reason setting the [`Console.InputEncoding`][0]
property on a customer's machine is throwing an `IOException`.
This causes Visual Studio (Windows) to completely crash:

	Description: The process was terminated due to an unhandled exception.
	Exception Info: System.IO.IOException
	   at System.IO.__Error.WinIOError(Int32, System.String)
	   at System.IO.__Error.WinIOError()
	   at System.Console.set_InputEncoding(System.Text.Encoding)
	   at Xamarin.Android.Tasks.Aapt2Daemon.Aapt2DaemonStart()
	   at System.Threading.ThreadHelper.ThreadStart_Context(System.Object)
	   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
	   at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
	   at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
	   at System.Threading.ThreadHelper.ThreadStart()

The cause of the crash is that commit 0d6fd83 sets
`Console.InputEncoding` to "UTF-8 without BOM" for communication with
`aapt2`, so that we can sanely use Unicode paths, and setting
`Console.InputEncoding` may result in a `System.IO.IOException`.
(Paths are written to `Process.StandardInput`, which can contain
Unicode characters.  `Console.InputEncoding` on the other hand,
defaults to "ANSI", which may not support full Unicode.)

Why did 0d6fd83 need to set `Console.InputEncoding` in the first
place?  Because [`ProcessStartInfo.StandardInputEncoding`][1] wasn't
added until .NET Standard 2.1, and .NET Framework <= 4.8 doesn't
support .NET Standard 2.1, only .NET Standard 2.0.  Furthermore,
when `ProcessStartInfo.RedirectStandardInput` is True,
[`Process.StandardInput` is a `StreamWriter`][2] which uses
`Console.InputEncoding` as the encoding for the underlying stream.
Thus, setting `Console.InputEncoding` is the only way in
.NET Standard 2.0/.NET Framework <= 4.8 to control the encoding used
by `Process.StandardInput`'s `StreamWriter`.

Further exploration since 0d6fd83 suggests that we could create a new
`StreamWriter` around `Process.StandardInput.BaseStream` with the
correct "UTF-8 without BOM" encoding, but that approach fails on CI:

	…\Xamarin.Android.Aapt2.targets(124,3): error APT0001: Unknown option `-o`. Please check `$(AndroidAapt2CompileExtraArgs)` and `$(AndroidAapt2LinkExtraArgs)` to see if they include any `aapt` command line arguments that are no longer valid for `aapt2` and ensure that all other arguments are valid for `aapt2`.
	…\Xamarin.Android.Aapt2.targets(124,3): error APT0001: Unknown option `-o`. Please check `$(AndroidAapt2CompileExtraArgs)` and `$(AndroidAapt2LinkExtraArgs)` to see if they include any `aapt` command line arguments that are no longer valid for `aapt2` and ensure that all other arguments are valid for `aapt2`.
	…\Xamarin.Android.Aapt2.targets(124,3): error APT0001: Unknown option `-o`. Please check `$(AndroidAapt2CompileExtraArgs)` and `$(AndroidAapt2LinkExtraArgs)` to see if they include any `aapt` command line arguments that are no longer valid for `aapt2` and ensure that all other arguments are valid for `aapt2`.
	…\Xamarin.Android.Aapt2.targets(124,3): error APT0001: Unknown option `-o`. Please check `$(AndroidAapt2CompileExtraArgs)` and `$(AndroidAapt2LinkExtraArgs)` to see if they include any `aapt` command line arguments that are no longer valid for `aapt2` and ensure that all other arguments are valid for `aapt2`.

We do not understand why this fails on CI unless
`Console.InputEncoding` is previously set.

All that said, we don't want Visual Studio to crash.

Fix the crash by:

 1. Using System.Reflection to probe for
    `ProcessStartInfo.StandardInputEncoding`.  If it exists, use it.

 2. If (2) fails, "fall back" and set `Console.InputEncoding` as
    before, wrapping in a `try`/`catch` block so that `IOException`s
    don't escape and crash Visual Studio.

    TODO: We need to figure out a way of reporting this exception
    somehow in the exception handler so we can get more data.

Regardless of whether (1) or (2) fails, as part of writing to `aapt2`s
`Process.StandardInput`, we wrap `Process.StandardInput.BaseStream`
into a new `StreamWriter` which uses "UTF-8 without BOM" as the
encoding.  This is "defense in depth," though could plausibly result
in the previously mentioned APT0001 errors if setting
`Console.InputEncoding` failed.

We'll see what happens "in the wild."

[0]: https://docs.microsoft.com/en-us/dotnet/api/system.console.inputencoding?view=netcore-3.1
[1]: https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.processstartinfo.standardinputencoding?view=netcore-3.1
[2]: https://referencesource.microsoft.com/#System/services/monitoring/system/diagnosticts/Process.cs,2154

Author: dellis1972

src/Xamarin.Android.Build.Tasks/Utilities/Aapt2Daemon.cs

@@ -5,8 +5,10 @@
 using System.IO;
 using System.Text;
 using System.Threading;
+using System.Reflection;
 using Microsoft.Build.Framework;
 using TPL = System.Threading.Tasks;
+using Xamarin.Android.Tools;
 
 namespace Xamarin.Android.Tasks
 {
@@ -144,6 +146,33 @@ public void Stop ()
 			pendingJobs.CompleteAdding ();
 		}
 
+		private bool SetConsoleInputEncoding (Encoding encoding)
+		{
+			try {
+				if (Console.InputEncoding != encoding) {
+					Console.InputEncoding = encoding;
+					return true;
+				}
+			} catch (IOException) {
+				//In a DesignTime Build on VS Windows sometimes this exception is raised.
+				//We should catch it, but there is nothing we can do about it.
+			}
+			return false;
+		}
+
+		private bool SetProcessInputEncoding (ProcessStartInfo info, Encoding encoding)
+		{
+			Type type = info.GetType ();
+			PropertyInfo prop = type.GetRuntimeProperty ("StandardInputEncoding");
+			if (prop == null)
+				prop = type.GetProperty ("StandardInputEncoding", BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance);
+			if(prop?.CanWrite ?? false) {
+				prop.SetValue (info, encoding, null);
+				return true;
+			}
+			return false;
+		}
+
 		private void Aapt2DaemonStart ()
 		{
 			ProcessStartInfo info = new ProcessStartInfo (Aapt2)
@@ -158,23 +187,24 @@ private void Aapt2DaemonStart ()
 				WorkingDirectory = Path.GetTempPath (),
 				StandardErrorEncoding = Encoding.UTF8,
 				StandardOutputEncoding = Encoding.UTF8,
+				// We need to FORCE the StandardInput to be UTF8 so we can use
+				// accented characters. Also DONT INCLUDE A BOM!!
+				// otherwise aapt2 will try to interpret the BOM as an argument.
 				// Cant use this cos its netstandard 2.1 only
 				// and we are using netstandard 2.0
-				//StandardInputEncoding = Encoding.UTF8,
+				//StandardInputEncoding = MonoAndroidHelper.UTF8withoutBOM,
 			};
-			// We need to FORCE the StandardInput to be UTF8 so we can use 
-			// accented characters. Also DONT INCLUDE A BOM!!
-			// otherwise aapt2 will try to interpret the BOM as an argument.
 			Process aapt2;
-			lock (lockObject) {
-				Encoding current = Console.InputEncoding;
+			Encoding currentEncoding = Console.InputEncoding;
+ 			lock (lockObject) {
 				try {
-					Console.InputEncoding = new UTF8Encoding (false);
+					if (!SetProcessInputEncoding (info, MonoAndroidHelper.UTF8withoutBOM))
+						SetConsoleInputEncoding (MonoAndroidHelper.UTF8withoutBOM);
 					aapt2 = new Process ();
 					aapt2.StartInfo = info;
 					aapt2.Start ();
 				} finally {
-					Console.InputEncoding = current;
+					SetConsoleInputEncoding (currentEncoding);
 				}
 			}
 			try {
@@ -183,15 +213,15 @@ private void Aapt2DaemonStart ()
 					bool errored = false;
 					try {
 						// try to write Unicode UTF8 to aapt2
-						StreamWriter writer = aapt2.StandardInput;
-						foreach (var arg in job.Commands)
-						{
-							writer.WriteLine (arg);
+						using (StreamWriter writer = new StreamWriter (aapt2.StandardInput.BaseStream, MonoAndroidHelper.UTF8withoutBOM, bufferSize: 1024, leaveOpen: true)) {
+							foreach (var arg in job.Commands) {
+								writer.WriteLine (arg);
+							}
+							writer.WriteLine ();
+							writer.Flush ();
 						}
-						writer.WriteLine ();
-						writer.Flush ();
 						string line;
-						
+
 						Queue<string> stdError = new Queue<string> ();
 						while ((line = aapt2.StandardError.ReadLine ()) != null) {
 							if (string.Compare (line, "Done", StringComparison.OrdinalIgnoreCase) == 0) {
@@ -201,8 +231,8 @@ private void Aapt2DaemonStart ()
 								errored = true;
 								continue;
 							}
-							// we have to queue the output because the "Done"/"Error" lines are 
-							//written after all the messages. So to process the warnings/errors 
+							// we have to queue the output because the "Done"/"Error" lines are
+							//written after all the messages. So to process the warnings/errors
 							// correctly we need to do this after we know if worked or failed.
 							stdError.Enqueue (line);
 						}
@@ -256,4 +286,4 @@ bool IsAapt2Warning (string singleLine)
 			return false;
 		}
 	}
-}
+}