Skip to content

Maximizing/Restoring when the alt buffer is active crashes conhost #1206

Closed

Description

  1. wsl
  2. printf '\e[?1049h'
  3. maximize
  4. did it crash? if not, restore
  5. did it crash? if not, maximize
  6. did it crash? if not, close this bug

Notably, the crash doesn't occur for simple resizes -- even those covering more than one side.

Activity

  1. added
    Product-ConhostFor issues in the Console codebase
    Area-OutputRelated to output processing (inserting text into buffer, retrieving buffer text, etc.)
    Issue-BugIt either shouldn't be doing this or needs an investigation.
    on Jun 11, 2019
  2. ghost added
    Needs-TriageIt's a new issue that the core contributor team needs to triage at the next triage meeting
    on Jun 11, 2019
  3. DHowett-MSFT commented on Jun 11, 2019

    @DHowett-MSFT
    ContributorAuthor
     	OpenConsole.exe!wil::details::in1diag5::_Throw_Hr(void * callerReturnAddress, unsigned int lineNumber, const char * fileName, const char * functionName, const char * code, HRESULT hr) Line 4937	C++
     	OpenConsole.exe!wil::details::in1diag5::Throw_HrIf(void * callerReturnAddress, unsigned int lineNumber, const char * fileName, const char * functionName, const char * code, HRESULT hr, bool condition) Line 5041	C++
     	OpenConsole.exe!TextBufferCellIterator::TextBufferCellIterator(const TextBuffer & buffer, _COORD pos, const Microsoft::Console::Types::Viewport limits) Line 43	C++
     	OpenConsole.exe!TextBuffer::GetCellDataAt(const _COORD at, const Microsoft::Console::Types::Viewport limit) Line 169	C++
     	OpenConsole.exe!Microsoft::Console::Render::Renderer::_PaintBufferOutput(Microsoft::Console::Render::IRenderEngine * const pEngine) Line 579	C++
     	OpenConsole.exe!Microsoft::Console::Render::Renderer::_PaintFrameForEngine(Microsoft::Console::Render::IRenderEngine * const pEngine) Line 119	C++
     	OpenConsole.exe!Microsoft::Console::Render::Renderer::PaintFrame() Line 68	C++
     	OpenConsole.exe!Microsoft::Console::Render::RenderThread::_ThreadProc() Line 168	C++
     	OpenConsole.exe!Microsoft::Console::Render::RenderThread::s_ThreadProc(void * lpParameter) Line 151	C++
    
  4. added
    Area-RenderingText rendering, emoji, complex glyph & font-fallback issues
    and removed
    Area-OutputRelated to output processing (inserting text into buffer, retrieving buffer text, etc.)
    on Jun 11, 2019
  5. zadjii-msft commented on Sep 3, 2019

    @zadjii-msft
    Member

    This only repros if "Disable scroll forward" is enabled.

    This surprises me, considering the alt buffer doesn't need to do anything with _virtualBottom, but something's definitely busted there.

  6. zadjii-msft commented on Sep 3, 2019

    @zadjii-msft
    Member

    This code looks bad: SCREEN_INFORMATION::_InternalSetViewportSize

    > OpenConsole!SCREEN_INFORMATION::_InternalSetViewportSize+0x5ca [...OpenConsole\src\host\screenInfo.cpp @ 1241] 
      OpenConsole!SCREEN_INFORMATION::_AdjustViewportSize+0x157 [...\OpenConsole\src\host\screenInfo.cpp @ 1297] 
      OpenConsole!SCREEN_INFORMATION::ProcessResizeWindow+0x14c [...\OpenConsole\src\host\screenInfo.cpp @ 887] 
      OpenConsole!Microsoft::Console::Interactivity::Win32::Window::_HandleWindowPosChanged+0x135 [...\OpenConsole\src\interactivity\win32\WindowProc.cpp @ 812] 
      OpenConsole!Microsoft::Console::Interactivity::Win32::Window::ConsoleWindowProc+0x101c [...\OpenConsole\src\interactivity\win32\WindowProc.cpp @ 478] 
    
        if (gci.IsTerminalScrolling() && newViewport.Height() != _viewport.Height())
        {
            const short newTop = static_cast<short>(std::max(0, _virtualBottom - (newViewport.Height() - 1)));
    
            newViewport = Viewport::FromDimensions(COORD({ newViewport.Left(), newTop }), newViewport.Dimensions());
        }
    

    During the resize down, we're resizing the alt buffer here. Prior to the newViewport = Viewport::FromDimensions(... line, members have the following values:

    Variable value
    newTop 76
    _virtualBottom 105
    newViewport {LT(0, 0) RB(119, 29) [120 x 30]}
    _viewport {LT(0, 0) RB(370, 105) [371 x 106]}

    Looks like we're trying to set the top of the new viewport so that the virtual bottom stays anchored to the new viewport's bottom. However, this only makes sense when the viewport is getting bigger, not when it's getting smaller. In that scenario, we'll end up setting the top to somewhere in the middle of the buffer, and then be real confused by the next paint.

  7. ghost added
    In-PRThis issue has a related PR
    on Sep 5, 2019
  8. added a commit that references this issue on Sep 5, 2019
    c58033c
  9. ghost added
    Needs-Tag-FixDoesn't match tag requirements
    on Sep 5, 2019
  10. ghost removed
    In-PRThis issue has a related PR
    on Sep 5, 2019
  11. ghost added
    Resolution-Fix-CommittedFix is checked in, but it might be 3-4 weeks until a release.
    on Sep 5, 2019
  12. ghost removed
    Needs-Tag-FixDoesn't match tag requirements
    on Sep 5, 2019
  13. added a commit that references this issue on Sep 23, 2019
    1284fca
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Area-RenderingText rendering, emoji, complex glyph & font-fallback issuesIssue-BugIt either shouldn't be doing this or needs an investigation.Priority-1A description (P1)Product-ConhostFor issues in the Console codebaseResolution-Fix-CommittedFix is checked in, but it might be 3-4 weeks until a release.Severity-CrashCrashes are real bad news.

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions