From 0ce654eaf648540348faea3f8a1c396c89e95a0e Mon Sep 17 00:00:00 2001
From: Leonard Hecker <lhecker@microsoft.com>
Date: Tue, 24 Sep 2024 21:06:01 +0200
Subject: [PATCH] Fix a cooked read deadlock (#17905)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 ✅
---
 src/buffer/out/textBuffer.cpp |  9 ++-------
 src/host/readDataCooked.cpp   | 34 ++++++++++++++++++++++++++++------
 2 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp
index 9e1e70e3d04..802e48e7c3e 100644
--- a/src/buffer/out/textBuffer.cpp
+++ b/src/buffer/out/textBuffer.cpp
@@ -451,14 +451,9 @@ 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;
@@ -466,7 +461,7 @@ size_t TextBuffer::FitTextIntoColumns(const std::wstring_view& chars, til::Coord
 
     // 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.
diff --git a/src/host/readDataCooked.cpp b/src/host/readDataCooked.cpp
index 117482f42e8..30515f7274d 100644
--- a/src/host/readDataCooked.cpp
+++ b/src/host/readDataCooked.cpp
@@ -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;
@@ -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;
+            }
         }
     }