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.

Sign up for GitHub

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

Jump to bottom

Snapping to smaller screen with a COOKED_READ crashes conpty #1856

Closed
2MuchLag4you opened this issue Jul 7, 2019 · 8 comments · Fixed by #5620
Closed

Snapping to smaller screen with a COOKED_READ crashes conpty #1856

2MuchLag4you opened this issue Jul 7, 2019 · 8 comments · Fixed by #5620
Assignees
@zadjii-msft
Labels
Area-Interaction Interacting with the vintage console window (as opposed to driving via API or hooks) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Conpty For console issues specifically related to conpty 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
Terminal v1.0

Comments

@2MuchLag4you
Copy link

Environment

Windows build number: [run "ver" at a command prompt]
**Microsoft Windows [Version 10.0.18362.207]**
OS language is dutch
Windows Terminal version (if applicable):
**Windows Terminal (Preview)
Version: 0.2.1831.0**

Any other software?

Steps to reproduce

full screen the terminal window and use the keys "windows + shift + arrow" to move the window to your second screen

Expected behavior

Go full screen on second monitor

Actual behavior

When I move it from a 1920x1080 screen to a 2560x1080 screen it goes full screen. When moving it from a 2560x1080 screen to a 1920x1080 screen the terminal crashes.

Message event viewer (Dutch)

Naam van toepassing met fout: WindowsTerminal.exe, versie: 1.0.1907.2001, tijdstempel: 0x5d1bd2d0
Naam van module met fout: ucrtbase.dll, versie: 10.0.18362.1, tijdstempel: 0x5cbddb81
Uitzonderingscode: 0xc0000409
Foutmarge: 0x000000000006d3be
Id van proces met fout: 0x3028
Starttijd van toepassing met fout: 0x01d534b9425f56b0
Pad naar toepassing met fout: C:\Program Files\WindowsApps\Microsoft.WindowsTerminal_0.2.1831.0_x64__8wekyb3d8bbwe\WindowsTerminal.exe
Pad naar module met fout: C:\WINDOWS\System32\ucrtbase.dll
Rapport-id: d822f0b0-3805-4c9f-b74e-87c80dc48334
Volledige pakketnaam met fout: Microsoft.WindowsTerminal_0.2.1831.0_x64__8wekyb3d8bbwe
Relatieve toepassings-id van pakket met fout: App
@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 Jul 7, 2019
@carlos-zamora carlos-zamora added Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Severity-Crash Crashes are real bad news. labels Jul 7, 2019
@carlos-zamora carlos-zamora changed the title Window Move Bug: "Win+Shift+Arrow" to smaller screen crashes Jul 7, 2019
@zadjii-msft
Copy link
Member

This seems like it's related to #1572, but since you're on 0.2.1831.0 (which includes the fix for that issue), it can't be the same root cause as that issue.

@zadjii-msft zadjii-msft added this to the Terminal v0.3 - Preview 2 milestone Jul 8, 2019
@zadjii-msft zadjii-msft added the Area-User Interface Issues pertaining to the user interface of the Console or Terminal label Jul 8, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jul 8, 2019
@DHowett-MSFT
Copy link
Contributor

This may be the same as #1795. We'll leave it open just to make sure.

@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 Jul 8, 2019
@zadjii-msft zadjii-msft mentioned this issue Jul 30, 2019
Merged
1 task
@zadjii-msft zadjii-msft self-assigned this Aug 14, 2019
@zadjii-msft
Copy link
Member

Largely, this issue's been fixed, but there's certainly another bug here, and it goes beyond the terminal.

If you create a conpty, and then you have a COOKED_READ with some input in that conpty, then snap the Terminal window to a smaller size like this, the conpty crashes. The conpty crashes when resizing, specifically due to the presence of the input line. I thought I had a fix in d34a3be for this, but I'm not sure that it totally works. It seems trivial enough, but knowing what I know about COOKED_READs, an easy fix is usually wrong. There's almost certainly edge cases it misses.

I want to leave this bug open to track that specific crash.

@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Aug 15, 2019
@zadjii-msft zadjii-msft reopened this Aug 15, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Aug 15, 2019
@zadjii-msft zadjii-msft modified the milestones: Terminal 1908.1, 20H1 Aug 15, 2019
@zadjii-msft zadjii-msft added Area-Interaction Interacting with the vintage console window (as opposed to driving via API or hooks) Product-Conpty For console issues specifically related to conpty and removed Area-User Interface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal. labels Aug 15, 2019
@zadjii-msft zadjii-msft changed the title Bug: "Win+Shift+Arrow" to smaller screen crashes Snapping to smaller screen with a COOKED_READ crashes conpty Aug 15, 2019
@zadjii-msft zadjii-msft removed their assignment Aug 15, 2019
@DHowett-MSFT DHowett-MSFT added the Priority-1 A description (P1) label Sep 5, 2019
@zadjii-msft zadjii-msft self-assigned this Sep 25, 2019
@galassie
Copy link

With v 6.0 is even worse.
Every time I go in full screen in other screen devices except the main one, it crashes.

This is the error logged in the event viewer:

Faulting application name: WindowsTerminal.exe, version: 1.0.1910.22001, time stamp: 0x5daf7ab2
Faulting module name: Windows.UI.Xaml.dll, version: 10.0.18362.418, time stamp: 0x253810c2
Exception code: 0xc000027b
Fault offset: 0x0000000000712dc0
Faulting process id: 0x3e58
Faulting application start time: 0x01d58a469cd74c0e
Faulting application path: C:\Program Files\WindowsApps\Microsoft.WindowsTerminal_0.6.2951.0_x64__8wekyb3d8bbwe\WindowsTerminal.exe
Faulting module path: C:\Windows\System32\Windows.UI.Xaml.dll
Report Id: 41431fbc-2508-4fde-af06-7e59afe73e85
Faulting package full name: Microsoft.WindowsTerminal_0.6.2951.0_x64__8wekyb3d8bbwe
Faulting package-relative application ID: App

@DHowett-MSFT
Copy link
Contributor

@galassie this looks unrelated (since the crash is in "Windows.UI.Xaml.dll" -- i bet that's #2277.

@galassie
Copy link

@galassie this looks unrelated (since the crash is in "Windows.UI.Xaml.dll" -- i bet that's #2277.

Yeah, you're right! I haven't seen that issue!

@cinnamon-msft cinnamon-msft modified the milestones: 20H1, 21H1 Nov 14, 2019
@DHowett-MSFT DHowett-MSFT mentioned this issue Dec 2, 2019
Closed
@zadjii-msft zadjii-msft mentioned this issue Dec 2, 2019
Closed
@zadjii-msft
Copy link
Member

Pulling this into 1.0 since we think it might actually be a pretty common crash

@zadjii-msft zadjii-msft modified the milestones: 21H1, Terminal v1.0 Apr 24, 2020
zadjii-msft added a commit that referenced this issue Apr 28, 2020
@zadjii-msft
This is the test for #1856
14fe652
zadjii-msft added a commit that referenced this issue Apr 28, 2020
@zadjii-msft
This is the fix for #1856
0d6dba5
This was referenced Apr 28, 2020
Closed
Merged
@ghost ghost added the In-PR This issue has a related PR label Apr 28, 2020
@ghost ghost closed this as completed in #5620 Apr 29, 2020
ghost pushed a commit that referenced this issue Apr 29, 2020
@zadjii-msft
Hide the commandline on a resize to prevent a crash when snapping the…
10fa310 
… window (#5620)

Hide any commandline (cooked read) we have before we begin a resize, and
show it again after the resize. 

## References

* I found #5618 while I was working on this.

## PR Checklist
* [x] Closes #1856
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated

## Detailed Description of the Pull Request / Additional comments

Basically, during a resize, we try to restore the viewport position
correctly, and part of that checks where the current commandline ends.
However, when we do that, the commandline's _current_ state still
reflects the _old_ buffer size, so resizing to be smaller can cause us
to throw an exception, when we find that the commandline doesn't fit in
the new viewport cleanly.

By hiding it, then redrawing it, we avoid this problem entirely. We
don't need to perform the check on the old commandline contents (since
they'll be empty), and we'll redraw it just fine for the new buffer size

## Validation Steps Performed
* ran tests
* checked resizing, snapping in conhost with a cooked read
* checked resizing, snapping in the Terminal with a cooked read
@ghost ghost added 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 Apr 29, 2020
@ghost
Copy link

ghost commented May 5, 2020

🎉This issue was addressed in #5620, which has now been successfully released as Windows Terminal Release Candidate v0.11.1251.0 (1.0rc1).:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zadjii-msft zadjii-msft

Labels
Area-Interaction Interacting with the vintage console window (as opposed to driving via API or hooks) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Conpty For console issues specifically related to conpty 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
Milestone
Terminal v1.0
Development

Successfully merging a pull request may close this issue.

Hide the commandline on a resize to prevent a crash when snapping the window microsoft/terminal
6 participants
@carlos-zamora @DHowett-MSFT @zadjii-msft @galassie @cinnamon-msft @2MuchLag4you