Move the VT parser's parse state into instanced storage (microsoft#3110)

The VT parser used to be keeping a boolean used to determine whether it
was in bulk or single-character parse mode in a function-level static.
That turned out to not be great.

Fixes microsoft#3108; fixes microsoft#3073.

Author: DHowett

src/terminal/parser/stateMachine.cpp

@@ -25,7 +25,8 @@ StateMachine::StateMachine(IStateMachineEngine* const pEngine) :
     // rgusParams Initialized below
     _sOscNextChar(0),
     _sOscParam(0),
-    _currRunLength(0)
+    _currRunLength(0),
+    _fProcessingIndividually(false)
 {
     ZeroMemory(_pwchOscStringBuffer, sizeof(_pwchOscStringBuffer));
     ZeroMemory(_rgusParams, sizeof(_rgusParams));
@@ -1346,20 +1347,16 @@ void StateMachine::ProcessString(const wchar_t* const rgwch, const size_t cch)
     _pwchSequenceStart = rgwch;
     _currRunLength = 0;
 
-    // This should be static, because if one string starts a sequence, and the next finishes it,
-    //   we want the partial sequence state to persist.
-    static bool s_fProcessIndividually = false;
-
     for (size_t cchCharsRemaining = cch; cchCharsRemaining > 0; cchCharsRemaining--)
     {
-        if (s_fProcessIndividually)
+        if (_fProcessingIndividually)
         {
             // If we're processing characters individually, send it to the state machine.
             ProcessCharacter(*_pwchCurr);
             _pwchCurr++;
             if (_state == VTStates::Ground) // Then check if we're back at ground. If we are, the next character (pwchCurr)
             { //   is the start of the next run of characters that might be printable.
-                s_fProcessIndividually = false;
+                _fProcessingIndividually = false;
                 _pwchSequenceStart = _pwchCurr;
                 _currRunLength = 0;
             }
@@ -1371,13 +1368,13 @@ void StateMachine::ProcessString(const wchar_t* const rgwch, const size_t cch)
                 FAIL_FAST_IF(!(_pwchSequenceStart + _currRunLength <= rgwch + cch));
                 _pEngine->ActionPrintString(_pwchSequenceStart, _currRunLength); // ... print all the chars leading up to it as part of the run...
                 _trace.DispatchPrintRunTrace(_pwchSequenceStart, _currRunLength);
-                s_fProcessIndividually = true; // begin processing future characters individually...
+                _fProcessingIndividually = true; // begin processing future characters individually...
                 _currRunLength = 0;
                 _pwchSequenceStart = _pwchCurr;
                 ProcessCharacter(*_pwchCurr); // ... Then process the character individually.
                 if (_state == VTStates::Ground) // If the character took us right back to ground, start another run after it.
                 {
-                    s_fProcessIndividually = false;
+                    _fProcessingIndividually = false;
                     _pwchSequenceStart = _pwchCurr + 1;
                     _currRunLength = 0;
                 }
@@ -1391,13 +1388,13 @@ void StateMachine::ProcessString(const wchar_t* const rgwch, const size_t cch)
     }
 
     // If we're at the end of the string and have remaining un-printed characters,
-    if (!s_fProcessIndividually && _currRunLength > 0)
+    if (!_fProcessingIndividually && _currRunLength > 0)
     {
         // print the rest of the characters in the string
         _pEngine->ActionPrintString(_pwchSequenceStart, _currRunLength);
         _trace.DispatchPrintRunTrace(_pwchSequenceStart, _currRunLength);
     }
-    else if (s_fProcessIndividually)
+    else if (_fProcessingIndividually)
     {
         // One of the "weird things" in VT input is the case of something like
         // <kbd>alt+[</kbd>. In VT, that's encoded as `\x1b[`. However, that's

src/terminal/parser/stateMachine.hpp

@@ -150,5 +150,9 @@ namespace Microsoft::Console::VirtualTerminal
         const wchar_t* _pwchCurr;
         const wchar_t* _pwchSequenceStart;
         size_t _currRunLength;
+
+        // This is tracked per state machine instance so that separate calls to Process*
+        //   can start and finish a sequence.
+        bool _fProcessingIndividually;
     };
 }

src/terminal/parser/ut_parser/Parser.UnitTests.vcxproj

@@ -12,6 +12,7 @@
   <ItemGroup>
     <ClCompile Include="InputEngineTest.cpp" />
     <ClCompile Include="OutputEngineTest.cpp" />
+    <ClCompile Include="StateMachineTest.cpp" />
     <ClCompile Include="..\precomp.cpp">
       <PrecompiledHeader>Create</PrecompiledHeader>
     </ClCompile>

src/terminal/parser/ut_parser/Parser.UnitTests.vcxproj.filters

@@ -15,7 +15,13 @@
     </Filter>
   </ItemGroup>
   <ItemGroup>
-    <ClCompile Include="stateMachineTest.cpp">
+    <ClCompile Include="InputEngineTest.cpp">
+      <Filter>Source Files</Filter>
+    </ClCompile>
+    <ClCompile Include="OutputEngineTest.cpp">
+      <Filter>Source Files</Filter>
+    </ClCompile>
+    <ClCompile Include="StateMachineTest.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
     <ClCompile Include="..\precomp.cpp">

src/terminal/parser/ut_parser/StateMachineTest.cpp

@@ -0,0 +1,113 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+#include "precomp.h"
+#include "WexTestClass.h"
+#include "../../inc/consoletaeftemplates.hpp"
+
+#include "stateMachine.hpp"
+
+using namespace WEX::Common;
+using namespace WEX::Logging;
+using namespace WEX::TestExecution;
+
+namespace Microsoft
+{
+    namespace Console
+    {
+        namespace VirtualTerminal
+        {
+            class StateMachineTest;
+            class TestStateMachineEngine;
+        };
+    };
+};
+
+using namespace Microsoft::Console::VirtualTerminal;
+
+class Microsoft::Console::VirtualTerminal::TestStateMachineEngine : public IStateMachineEngine
+{
+public:
+    bool ActionExecute(const wchar_t /* wch */) override { return true; };
+    bool ActionExecuteFromEscape(const wchar_t /* wch */) override { return true; };
+    bool ActionPrint(const wchar_t /* wch */) override { return true; };
+    bool ActionPrintString(const wchar_t* const /* rgwch */,
+                           size_t const /* cch */) override { return true; };
+
+    bool ActionPassThroughString(const wchar_t* const /* rgwch */,
+                                 size_t const /* cch */) override { return true; };
+
+    bool ActionEscDispatch(const wchar_t /* wch */,
+                           const unsigned short /* cIntermediate */,
+                           const wchar_t /* wchIntermediate */) override { return true; };
+
+    bool ActionClear() override { return true; };
+
+    bool ActionIgnore() override { return true; };
+
+    bool ActionOscDispatch(const wchar_t /* wch */,
+                           const unsigned short /* sOscParam */,
+                           wchar_t* const /* pwchOscStringBuffer */,
+                           const unsigned short /* cchOscString */) override { return true; };
+
+    bool ActionSs3Dispatch(const wchar_t /* wch */,
+                           const unsigned short* const /* rgusParams */,
+                           const unsigned short /* cParams */) override { return true; };
+
+    bool FlushAtEndOfString() const override { return false; };
+    bool DispatchControlCharsFromEscape() const override { return false; };
+    bool DispatchIntermediatesFromEscape() const override { return false; };
+
+    // ActionCsiDispatch is the only method that's actually implemented.
+    bool ActionCsiDispatch(const wchar_t /* wch */,
+                           const unsigned short /* cIntermediate */,
+                           const wchar_t /* wchIntermediate */,
+                           _In_reads_(cParams) const unsigned short* const rgusParams,
+                           const unsigned short cParams) override
+    {
+        csiParams.emplace(rgusParams, rgusParams + cParams);
+        return true;
+    }
+
+    // This will only be populated if ActionCsiDispatch is called.
+    std::optional<std::vector<unsigned short>> csiParams;
+};
+
+class Microsoft::Console::VirtualTerminal::StateMachineTest
+{
+    TEST_CLASS(StateMachineTest);
+
+    TEST_CLASS_SETUP(ClassSetup)
+    {
+        return true;
+    }
+
+    TEST_CLASS_CLEANUP(ClassCleanup)
+    {
+        return true;
+    }
+
+    TEST_METHOD(TwoStateMachinesDoNotInterfereWithEachother);
+};
+
+void StateMachineTest::TwoStateMachinesDoNotInterfereWithEachother()
+{
+    auto firstEnginePtr{ std::make_unique<TestStateMachineEngine>() };
+    // this dance is required because StateMachine presumes to take ownership of its engine.
+    const auto& firstEngine{ *firstEnginePtr.get() };
+    StateMachine firstStateMachine{ firstEnginePtr.release() };
+
+    auto secondEnginePtr{ std::make_unique<TestStateMachineEngine>() };
+    const auto& secondEngine{ *secondEnginePtr.get() };
+    StateMachine secondStateMachine{ secondEnginePtr.release() };
+
+    firstStateMachine.ProcessString(L"\x1b[12"); // partial sequence
+    secondStateMachine.ProcessString(L"\x1b[3C"); // full sequence on second parser
+    firstStateMachine.ProcessString(L";34m"); // completion to previous partial sequence on first parser
+
+    std::vector<unsigned short> expectedFirstCsi{ 12u, 34u };
+    std::vector<unsigned short> expectedSecondCsi{ 3u };
+
+    VERIFY_ARE_EQUAL(expectedFirstCsi, firstEngine.csiParams);
+    VERIFY_ARE_EQUAL(expectedSecondCsi, secondEngine.csiParams);
+}

src/terminal/parser/ut_parser/sources

@@ -23,6 +23,7 @@ SOURCES = \
     $(SOURCES) \
     OutputEngineTest.cpp \
     InputEngineTest.cpp \
+    StateMachineTest.cpp \
 
 # The InputEngineTest requires VTRedirMapVirtualKeyW, which means we need the
 # ServiceLocator, which means we need the entire host and all it's dependencies,