[Xamarin.Android.Build.Tasks] Catch Exception when setting Console.InputEncoding

Documentation/release-notes/4722.md

@@ -0,0 +1,24 @@
+#### Visual Studio Crashes when changing an AndroidResource file.
+
+- [Developer Community 1038779](https://developercommunity.visualstudio.com/content/problem/1038779/visual-studio-crash-down.html):
+- [GitHub 4734](https://github.com/xamarin/xamarin-android/issues/4734):
+
+In 16.7 Preview 1 we encountered a problem were Visual Studio world
+crash uneexpectedly. Further investigation lead to an Unhandled `IOException`
+in the new `aapt2` deamon code causing the problem. This only seems to
+happen when `AndroidUseManagedDesignTimeResourceGenerator` is set to
+`False` in the csproj. As to why it happens that is still unknown. The
+problem occurs when we are trying to set the `InputEncoding` of the `Console`.
+This is to make sure `aapt2` can handle non-ASCII characters in paths. However
+when running in a design time build the `IOException` of `invalid handle` is thrown.
+This does not occur during a normal build.
+
+The workarond is to remove the `AndroidUseManagedDesignTimeResourceGenerator` from
+the csproj. The fix is to catch the `IOException` and allow `aapt2` to run. This will
+stop Visual Studio from crashing.
+
+### Known issues
+
+  If the `IOException` is thrown and caught, `aapt2` will not be able to accept non-ASCII
+  characters for paths as its input. You might see `aapt2` build failures if you have
+  non-ASCII characters in your paths.

src/Xamarin.Android.Build.Tasks/Tasks/Aapt2.cs

@@ -18,7 +18,7 @@
 using Xamarin.Build;
 
 namespace Xamarin.Android.Tasks {
-	
+
 	public abstract class Aapt2 : AndroidAsyncTask {
 
 		private static readonly int DefaultMaxAapt2Daemons = 6;
@@ -57,15 +57,15 @@ protected string GenerateFullPathToTool ()
 		protected virtual int GetRequiredDaemonInstances ()
 		{
 			return 1;
-		} 
+		}
 
 		Aapt2Daemon daemon;
 
 		internal Aapt2Daemon Daemon => daemon;
 		public override bool Execute ()
 		{
 			// Must register on the UI thread!
-			// We don't want to use up ALL the available cores especially when 
+			// We don't want to use up ALL the available cores especially when
 			// running in the IDE. So lets cap it at DefaultMaxAapt2Daemons (6).
 			int maxInstances = Math.Min (Environment.ProcessorCount-1, DefaultMaxAapt2Daemons);
 			if (DaemonMaxInstanceCount == 0)
@@ -74,6 +74,9 @@ public override bool Execute ()
 				DaemonMaxInstanceCount = Math.Min (DaemonMaxInstanceCount, maxInstances);
 			daemon  = Aapt2Daemon.GetInstance (BuildEngine4, GenerateFullPathToTool (),
 				DaemonMaxInstanceCount, GetRequiredDaemonInstances (), registerInDomain: DaemonKeepInDomain);
+			while (daemon.StartupWarnings.Count > 0) {
+				LogCodedWarning ("APT2000", daemon.StartupWarnings.Dequeue ());
+			}
 			return base.Execute ();
 		}
 
@@ -97,7 +100,7 @@ protected void ProcessOutput ()
 					}
 				}
 			}
-		} 
+		}
 
 		protected bool LogAapt2EventsFromOutput (string singleLine, MessageImportance messageImportance, bool apptResult)
 		{
@@ -122,7 +125,7 @@ protected bool LogAapt2EventsFromOutput (string singleLine, MessageImportance me
 					return true;
 				}
 				if (message.StartsWith ("unknown option", StringComparison.OrdinalIgnoreCase)) {
-					// we need to filter out the remailing help lines somehow. 
+					// we need to filter out the remailing help lines somehow.
 					LogCodedError ("APT0001", Properties.Resources.APT0001, message.Substring ("unknown option '".Length).TrimEnd ('.', '\''));
 					return false;
 				}

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

@@ -57,6 +57,7 @@ public void Complete (bool result)
 		long jobsRunning = 0;
 		long jobId = 0;
 		int maxInstances = 0;
+		Queue<string> daemonStartupWarnings = new Queue<string> ();
 
 		public CancellationToken Token => tcs.Token;
 
@@ -77,6 +78,8 @@ public bool JobsRunning
 
 		public int CurrentInstances => daemons.Count;
 
+		public Queue<string> StartupWarnings => daemonStartupWarnings;
+
 		public Aapt2Daemon (string aapt2, int maxNumberOfInstances, int initalNumberOfDaemons)
 		{
 			Aapt2 = aapt2;
@@ -162,21 +165,34 @@ private void Aapt2DaemonStart ()
 				// and we are using netstandard 2.0
 				//StandardInputEncoding = Encoding.UTF8,
 			};
-			// We need to FORCE the StandardInput to be UTF8 so we can use 
+			// 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;
+			Process aapt2 = null;
 			lock (lockObject) {
-				Encoding current = Console.InputEncoding;
 				try {
-					Console.InputEncoding = new UTF8Encoding (false);
-					aapt2 = new Process ();
-					aapt2.StartInfo = info;
-					aapt2.Start ();
-				} finally {
-					Console.InputEncoding = current;
+					Encoding current = Console.InputEncoding;
+					try {
+						try {
+							Console.InputEncoding = new UTF8Encoding (false);
+						} catch (IOException ioEx) {
+							// For some reason we can not set the InputEncoding. If this happens we don't
+							// want to crash Visual Studio with an Unhandled Exception.
+							// The downside of ignoring this is paths with accented characters will cause problems.
+							daemonStartupWarnings.Enqueue (ioEx.ToString ());
+						}
+						aapt2 = new Process ();
+						aapt2.StartInfo = info;
+						aapt2.Start ();
+					} finally {
+						Console.InputEncoding = current;
+					}
+				} catch (Exception ex) {
+					daemonStartupWarnings.Enqueue (ex.ToString ());
 				}
 			}
+			if (aapt2 == null)
+				return;
 			try {
 				foreach (var job in pendingJobs.GetConsumingEnumerable (tcs.Token)) {
 					Interlocked.Add (ref jobsRunning, 1);
@@ -191,7 +207,7 @@ private void Aapt2DaemonStart ()
 						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 +217,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 +272,4 @@ bool IsAapt2Warning (string singleLine)
 			return false;
 		}
 	}
-}
+}