diff --git a/Documentation/release-notes/4722.md b/Documentation/release-notes/4722.md new file mode 100644 index 00000000000..181e744b091 --- /dev/null +++ b/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. \ No newline at end of file diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/Aapt2.cs b/src/Xamarin.Android.Build.Tasks/Tasks/Aapt2.cs index 55531889be4..6e1a29c42e3 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/Aapt2.cs +++ b/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,7 +57,7 @@ protected string GenerateFullPathToTool () protected virtual int GetRequiredDaemonInstances () { return 1; - } + } Aapt2Daemon daemon; @@ -65,7 +65,7 @@ protected virtual int GetRequiredDaemonInstances () 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; } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/Aapt2Daemon.cs b/src/Xamarin.Android.Build.Tasks/Utilities/Aapt2Daemon.cs index 3e672418963..a30ada32185 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/Aapt2Daemon.cs +++ b/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 daemonStartupWarnings = new Queue (); public CancellationToken Token => tcs.Token; @@ -77,6 +78,8 @@ public bool JobsRunning public int CurrentInstances => daemons.Count; + public Queue 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 stdError = new Queue (); 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; } } -} \ No newline at end of file +}