Merged PR 5903250: [Git2Git] Merged PR 5895926: conhost: fix two mode…

…rately high-hitting Watsons

This commit fixes two issues:

1. We were pushing ConsoleWaitBlocks into the wait queue before they
   were fully constructed. This resulted in the wait being called before
   it even had an API message in it. [MSFT-24113101]
2. Drawing DBCS characters and resizing (a lot) would cause a crash
   because of an invalid DBCS cell state. This hits in both Terminal and
   conhost. [MSFT-17364373] [GH-4907]

The DBCS state check was promoted from an NT_ASSERT (which never fired
in release) to a FAIL_FAST in !1794053. The console kept chugging along
without failing in fre for all those years.

Fixes MSFT-24113101
Fixes MSFT-17364373
Fixes GH-4907 Retrieved from https://microsoft.visualstudio.com os.2020 OS official/rs_wdx_dxp_windev 108e746630749aa7851dd813b19e013ae31ef0db

(cherry picked from commit 8e7a866)

src/buffer/out/textBuffer.cpp

@@ -281,9 +281,9 @@ bool TextBuffer::_AssertValidDoubleByteSequence(const DbcsAttribute dbcsAttribut
 // - false otherwise (out of memory)
 bool TextBuffer::_PrepareForDoubleByteSequence(const DbcsAttribute dbcsAttribute)
 {
-    // Assert the buffer state is ready for this character
-    // This function corrects most errors. If this is false, we had an uncorrectable one.
-    FAIL_FAST_IF(!(_AssertValidDoubleByteSequence(dbcsAttribute))); // Shouldn't be uncorrectable sequences unless something is very wrong.
+    // This function corrects most errors. If this is false, we had an uncorrectable one which
+    // older versions of conhost simply let pass by unflinching.
+    LOG_HR_IF(E_NOT_VALID_STATE, !(_AssertValidDoubleByteSequence(dbcsAttribute))); // Shouldn't be uncorrectable sequences unless something is very wrong.
 
     bool fSuccess = true;
     // Now compensate if we don't have enough space for the upcoming double byte sequence

src/server/WaitBlock.cpp

@@ -15,8 +15,8 @@
 
 // Routine Description:
 // - Initializes a ConsoleWaitBlock
-// - ConsoleWaitBlocks will self-manage their position in their two queues.
-// - They will push themselves into the tail and store the iterator for constant deletion time later.
+// - ConsoleWaitBlocks will mostly self-manage their position in their two queues.
+// - They will be pushed into the tail and the resulting iterator stored for constant deletion time later.
 // Arguments:
 // - pProcessQueue - The queue attached to the client process ID that requested this action
 // - pObjectQueue - The queue attached to the console object that will service the action when data arrives
@@ -30,9 +30,6 @@ ConsoleWaitBlock::ConsoleWaitBlock(_In_ ConsoleWaitQueue* const pProcessQueue,
     _pObjectQueue(THROW_HR_IF_NULL(E_INVALIDARG, pObjectQueue)),
     _pWaiter(THROW_HR_IF_NULL(E_INVALIDARG, pWaiter))
 {
-    _itProcessQueue = _pProcessQueue->_blocks.insert(_pProcessQueue->_blocks.end(), this);
-    _itObjectQueue = _pObjectQueue->_blocks.insert(_pObjectQueue->_blocks.end(), this);
-
     _WaitReplyMessage = *pWaitReplyMessage;
 
     // We will write the original message back (with updated out parameters/payload) when the request is finally serviced.
@@ -87,6 +84,10 @@ ConsoleWaitBlock::~ConsoleWaitBlock()
                                           pObjectQueue,
                                           pWaitReplyMessage,
                                           pWaiter);
+
+        // Set the iterators on the wait block so that it can remove itself later.
+        pWaitBlock->_itProcessQueue = pProcessQueue->_blocks.insert(pProcessQueue->_blocks.end(), pWaitBlock);
+        pWaitBlock->_itObjectQueue = pObjectQueue->_blocks.insert(pObjectQueue->_blocks.end(), pWaitBlock);
     }
     catch (...)
     {