Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Writing random output to console output handle fails with no last error set #3320

Closed
benhillis opened this issue Oct 24, 2019 · 5 comments · Fixed by #3380
Closed

Writing random output to console output handle fails with no last error set #3320

benhillis opened this issue Oct 24, 2019 · 5 comments · Fixed by #3380
Assignees
Labels
Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-1 A description (P1) Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Crash Crashes are real bad news.
Milestone

Comments

@benhillis
Copy link
Member

Environment

Windows build number: Microsoft Windows [Version 10.0.19002.1000]

Any other software?
WSL2

Steps to reproduce

microsoft/WSL#4495

  1. Install Ubuntu 18.04 distro from store
  2. for i in /bin/*; do head ${i}; done
  3. Observe wsl window hang or crash

Expected behavior

Contents of the files are displayed, no hang / crash.

Actual behavior

WSL window will close, or hang. Seems like an issue with (or around) Utf8ToWideCharParser::_InvolvedParse.

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Oct 24, 2019
@zadjii-msft zadjii-msft added Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Conhost For issues in the Console codebase Severity-Crash Crashes are real bad news. labels Oct 25, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Oct 25, 2019
@zadjii-msft zadjii-msft added this to the 20H1 milestone Oct 25, 2019
@jstarks
Copy link
Member

jstarks commented Oct 28, 2019

Should _InvolvedParse just stop passing MB_ERR_INVALID_CHARS to MultiByteToWideChar?

@benhillis
Copy link
Member Author

@jstarks - that was my naive take, I can try building a private with that change.

@jstarks
Copy link
Member

jstarks commented Oct 28, 2019

Although there's also a call to _RemoveInvalidSequences before that, which presumably should remove any invalid UTF-8 sequences.

@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Oct 28, 2019
@DHowett-MSFT
Copy link
Contributor

(This is being investigated.)

@ghost ghost added the In-PR This issue has a related PR label Oct 30, 2019
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Oct 31, 2019
@ghost
Copy link

ghost commented Nov 26, 2019

🎉This issue was addressed in #3380, which has now been successfully released as Windows Terminal Preview v0.7.3291.0.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-1 A description (P1) Product-Conhost For issues in the Console codebase Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Crash Crashes are real bad news.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants