Skip to content

Conversation

@dellis1972
Copy link
Contributor

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

For some unknown reason the setting of Console.InputEncoding
on a customers machine is throwing an IOException. This then
causes Visual Studio 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()

At this time we are still unsure as to what is causing the problem.
But we should at least catch the exception to that Visual Studio does
not crash.

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

Note: Not sure what is going on with the line ending changes :/

} 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we end up with this PR as intermediate fix until a complete solution is found, then when you get a chance, if you could whip up a little known issue that describes the symptom users would see when using an accented character, that would be perfect.

Fingers crossed that the complete solution is just over the horizon though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The known issue can go alongside the draft release note for this PR in a Documentation/release-notes/4722.md file as part of this PR, so something like:

### IDE compatibility

- [Developer Community 1038779](https://developercommunity.visualstudio.com/content/problem/1038779/visual-studio-crash-down.html):
  ...

### Known issues

- [GitHub nnnn]():
  ... prevents building successfully if project paths contain accented
  characters or other non-ASCII UTF8 characters for projects configured to use
  AAPT2.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brendanzagaeski I can't get the actual error they might see as we have a bug stopping me from building in a directory with accented characters

@dellis1972
Copy link
Contributor Author

More info on the error

Xamarin.VisualStudio.UnhandledExceptionsManager|Error|0|An unhandled error occurred. Details: The handle is invalid.


System.IO.IOException: The handle is invalid.

   at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
   at System.IO.__Error.WinIOError()
   at System.Console.set_InputEncoding(Encoding value)
   at Xamarin.Android.Tasks.Aapt2Daemon.Aapt2DaemonStart()
   at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ThreadHelper.ThreadStart()

The error is handle is invalid 🤷 I have no idea why the Console handle would be invalid.

For some unknown reason the setting of `Console.InputEncoding`
on a customers machine is throwing an IOException. This then
causes Visual Studio 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()
```

At this time we are still unsure as to what is causing the problem.
But we should at least catch the exception to that Visual Studio does
not crash.

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

Note: Not sure what is going on with the line ending changes :/
@dellis1972 dellis1972 marked this pull request as ready for review May 27, 2020 14:23
daemonStartupWarnings.Enqueue (ex.ToString ());
}
}
if (aapt2 == null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can this happen? Line 184 will re-assign aapt2 to a non-null value, unless the Process constructor throws.

If aapt2 could be null, that sounds like a possible "semantic" violation, as other parts of the code will assume/require that aapt2 be running, but if it isn't running, what'll happen?

Console.InputEncoding = current;
}
} catch (Exception ex) {
daemonStartupWarnings.Enqueue (ex.ToString ());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can't start the daemon at all here, should this fail instead?

It seems like the warning would be OK for IOException but not in the case where it couldn't start the daemon at all?

@jonpryor
Copy link
Contributor

In retrospect, I really don't like the idea of setting Console.InputEncoding in the first place; it's a global solution for a local problem, the local problem being that ProcessStartInfo.StandardInputEncoding not being part of .NET Standard 2.0.

We should either use .NET Standard 2.1 (PR #4735, which apparently is failing), or use System.Reflection to lookup and set ProcessStartInfo.StandardInputEncoding, which is a local solution for a local problem.

@dellis1972 dellis1972 added the do-not-merge PR should not be merged. label May 28, 2020
@dellis1972 dellis1972 closed this Jun 1, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

do-not-merge PR should not be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants