Skip to content

Commit

Permalink
Fix a cooked read deadlock (#17905)
Browse files Browse the repository at this point in the history
Because `_layoutLine` would never return `column == columnLimit` for
control character visualizers, we'd get a deadlock in `_redisplay`,
as it tries to fill the line until it's full, but never achieve it.

Closes #17893

## Validation Steps Performed
* Press Ctrl-A to insert "^A"
* Press Home to get to the start of the prompt
* Press and hold "A" until the line wraps
* The line wraps and there's no deadlock ✅
  • Loading branch information
lhecker authored Sep 24, 2024
1 parent fc586e2 commit 0ce654e
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 13 deletions.
9 changes: 2 additions & 7 deletions src/buffer/out/textBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -451,22 +451,17 @@ size_t TextBuffer::FitTextIntoColumns(const std::wstring_view& chars, til::Coord
cwd.GraphemeNext(state, chars);
col += state.width;

// If we ran out of columns, we need to always return `columnLimit` and not `cols`,
// because if we tried inserting a wide glyph into just 1 remaining column it will
// fail to fit, but that remaining column still has been used up. When the caller sees
// `columns == columnLimit` they will line-wrap and continue inserting into the next row.
if (col > columnLimit)
{
columns = columnLimit;
return dist;
break;
}

dist += state.len;
}

// But if we simply ran out of text we just need to return the actual number of columns.
columns = col;
return chars.size();
return dist;
}

// Pretend as if `position` is a regular cursor in the TextBuffer.
Expand Down
34 changes: 28 additions & 6 deletions src/host/readDataCooked.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1267,17 +1267,30 @@ COOKED_READ_DATA::LayoutResult COOKED_READ_DATA::_layoutLine(std::wstring& outpu
output.append(text, 0, len);
column += cols;
it += len;
}

const auto nextPlainChar = std::find_if(it, end, [](const auto& wch) { return wch >= L' '; });
for (; it != nextPlainChar; ++it)
{
if (column >= columnLimit)
if (it != nextControlChar)
{
// The only reason that not all text could be fit into the line is if the last character was a wide glyph.
// In that case we want to return the columnLimit, to indicate that the row is full and a line wrap is required,
// BUT DON'T want to pad the line with a whitespace to actually fill the line to the columnLimit.
// This is because copying the prompt contents (Ctrl-A, Ctrl-C) should not copy any trailing padding whitespace.
//
// Thanks to this lie, the _redisplay() code will not use a CRLF sequence or similar to move to the next line,
// as it thinks that this row has naturally wrapped. This causes it to print the wide glyph on the preceding line
// which causes the terminal to insert the padding whitespace for us.
column = columnLimit;
goto outerLoopExit;
break;
}

if (column >= columnLimit)
{
break;
}
}

const auto nextPlainChar = std::find_if(it, end, [](const auto& wch) { return wch >= L' '; });
for (; it != nextPlainChar; ++it)
{
const auto wch = *it;
wchar_t buf[8];
til::CoordType len = 0;
Expand All @@ -1297,11 +1310,20 @@ COOKED_READ_DATA::LayoutResult COOKED_READ_DATA::_layoutLine(std::wstring& outpu

if (column + len > columnLimit)
{
// Unlike above with regular text we can't avoid padding the line with whitespace, because a string
// like "^A" is not a wide glyph, and so we cannot trick the terminal to insert the padding for us.
output.append(columnLimit - column, L' ');
column = columnLimit;
goto outerLoopExit;
}

output.append(buf, len);
column += len;

if (column >= columnLimit)
{
goto outerLoopExit;
}
}
}

Expand Down

0 comments on commit 0ce654e

Please sign in to comment.