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

Resizing the font while playing a sixel video can trigger a crash #17947

Closed
j4james opened this issue Sep 21, 2024 · 0 comments · Fixed by #17951
Closed

Resizing the font while playing a sixel video can trigger a crash #17947

j4james opened this issue Sep 21, 2024 · 0 comments · Fixed by #17951
Labels
In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting

Comments

@j4james
Copy link
Collaborator

j4james commented Sep 21, 2024

Windows Terminal version

1.22.2362.0

Windows build number

10.0.19045.4894

Other Software

img2sixel

Steps to reproduce

  1. Start a WSL shell in Windows Terminal.
  2. Get some content in the buffer so that resizing will force a reflow.
  3. View an animated gif with img2sixel filename.gif so you've got some sixel content being constantly refreshed.
  4. Resize the font with the mouse scroll wheel, so you're rapidly growing and shrinking the animation. Make sure the image sometimes grows larger than the viewport.

Expected Behavior

The animation should continue to play without any problems, aside from the fact that some partial frames will be pushed into the scrollback buffer when the image becomes too large to fit the height of the page.

Actual Behavior

The terminal eventually crashes.

This doesn't happen in OpenConsole, but that is assumedly because it resizes the window when you change the font size, so the text dimensions never change, and thus no reflow is required. Although now that I say that, I suspect you could possibly cause it to crash just by resizing the window in a way that triggers a reflow.
@j4james j4james added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Sep 21, 2024
@j4james j4james mentioned this issue Sep 22, 2024
Merged
1 task
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Sep 22, 2024
DHowett pushed a commit that referenced this issue Sep 24, 2024
@j4james @DHowett
Fix a sixel crash when the buffer is reflowed (#17951)
145babe 
## Summary of the Pull Request

The sixel parser has an internal buffer that holds the indexed-color
representation of the image, prior to it being translated to RGB. This
buffer only retains the section of the image that is within the visible
viewport, so we're continually erasing segments from the top of it when
the image is large enough to trigger a scroll.

But there is a problem that arises if the window or font is resized so
that the buffer needs to reflow, because that can result in the image
being pushed entirely offscreen. At that point the segment we're trying
to erase is actually larger than the buffer itself, which can end up
causing the terminal to crash

To fix this, we just need to check for an oversized erase attempt and
simply clear the buffer instead.

## Validation Steps Performed

I could easily reproduce this crash in Windows Terminal by resizing the
font while viewing an animated gif with img2sixel. With this PR applied
the crash no longer occurs.

## PR Checklist
- [x] Closes #17947

(cherry picked from commit fc586e2)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgTTKv4
Service-Version: 1.22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix a sixel crash when the buffer is reflowed j4james/terminal
1 participant
@j4james