Skip to content

Commit

Permalink
Fix VtEngine hang when resizing while scrolling (#15618)
Browse files Browse the repository at this point in the history
This fixes a bug reported internally that occurs when resizing the
terminal while also scolling the contents. The easiest way to reproduce
it is to resize the terminal to 0 rows, but it's much more prominent
in a debug build where everything goes out of sync almost immediately.

The underlying issue is that `VtEngine::_wrappedRow` may contain an
offset that is outside of the viewport bounds, because reflowing and
scrolling aren't properly synchronized. The previous `bitmap` code
would then throw an exception for such invalid coordinates and cause
the internal `VtEngine` state to be broken. Once `_wrappedRow` got
to a negative value at least once, it would stay that way unless you're
scrolling up. If the contents are actively scrolling it would quickly
reach a negative value from which it can never recover. At that point
OpenConsole would enter a tight exception-throw-catch-retry loop
and Windows Terminal seemingly cease to show any content.

## Validation Steps Performed
* Resize WT to the minimal window size repeatedly
* Doesn't hang ✅
  • Loading branch information
lhecker authored Jun 29, 2023
1 parent 80f2776 commit 358e10b
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 22 deletions.
22 changes: 14 additions & 8 deletions src/inc/til/bitmap.h
Original file line number Diff line number Diff line change
Expand Up @@ -351,20 +351,26 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"

void set(const til::point pt)
{
THROW_HR_IF(E_INVALIDARG, !_rc.contains(pt));
_runs.reset(); // reset cached runs on any non-const method

_bits.set(_rc.index_of(pt));
if (_rc.contains(pt))
{
_runs.reset(); // reset cached runs on any non-const method
_bits.set(_rc.index_of(pt));
}
}

void set(const til::rect& rc)
void set(til::rect rc)
{
THROW_HR_IF(E_INVALIDARG, !_rc.contains(rc));
_runs.reset(); // reset cached runs on any non-const method

for (auto row = rc.top; row < rc.bottom; ++row)
rc &= _rc;

const auto width = rc.width();
const auto stride = _rc.width();
auto idx = _rc.index_of({ rc.left, rc.top });

for (auto row = rc.top; row < rc.bottom; ++row, idx += stride)
{
_bits.set(_rc.index_of(til::point{ rc.left, row }), rc.width(), true);
_bits.set(idx, width, true);
}
}

Expand Down
21 changes: 7 additions & 14 deletions src/til/ut_til/BitmapTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -632,26 +632,19 @@ class BitmapTests
_checkBits(expectedSet, bitmap);
}

TEST_METHOD(SetResetExceptions)
TEST_METHOD(SetResetOutOfBounds)
{
til::bitmap map{ til::size{ 4, 4 } };
Log::Comment(L"1.) SetPoint out of bounds.");
{
auto fn = [&]() {
map.set(til::point{ 10, 10 });
};

VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; });
}
map.set(til::point{ 10, 10 });

Log::Comment(L"2.) SetRectangle out of bounds.");
{
auto fn = [&]() {
map.set(til::rect{ til::point{ 2, 2 }, til::size{ 10, 10 } });
};
map.set(til::rect{ til::point{ 2, 2 }, til::size{ 10, 10 } });

VERIFY_THROWS_SPECIFIC(fn(), wil::ResultException, [](wil::ResultException& e) { return e.GetErrorCode() == E_INVALIDARG; });
}
const auto runs = map.runs();
VERIFY_ARE_EQUAL(2u, runs.size());
VERIFY_ARE_EQUAL(til::rect(2, 2, 4, 3), runs[0]);
VERIFY_ARE_EQUAL(til::rect(2, 3, 4, 4), runs[1]);
}

TEST_METHOD(Resize)
Expand Down

0 comments on commit 358e10b

Please sign in to comment.