Fix "Ctrl+Break can leave the WSL shell in an unusable state"

[WhatsACloud]

Summary of the Pull Request

Runs reset in the shell after user kills the shell with ctrl + break and presses enter to restart the terminal.

References and Relevant Issues

18425

Detailed Description of the Pull Request / Additional comments

reset resets the terminal state, preventing the state of the previous terminal session carrying over into the next one and hence any unintended terminal behavior such as that shown in issue 18425.

Validation Steps Performed

Reproduce the bug in 18425. It should no longer occur.

PR Checklist

• Closes #xxx
• Tests added/passed
• Documentation updated
  • If checked, please file a pull request on our docs repo and link it here: #xxx
• Schema updated (if necessary)

[DHowett]

Thanks so much for this!

I have some concerns about the overall approach - right now, it simulates the user typing in reset any time the connection gets restarted. That may work for most connections to Linux machines, but won't work for Command Prompt, PowerShell, Yori, or some of the more esoteric things people run inside Terminal:

On Linuxen, reset usually causes the client application to emit a hard reset (or a soft reset!) VT sequence as the output. I wonder: would it be possible or desirable to simulate that, instead?

Rather than requesting that it run the reset command, you could output a set of control sequences that restore the terminal session to sanity.

Another approach could be to use some of the internal APIs between ControlCore and TerminalCore to reinitialize the terminal state to the default, rather than emitting output or input at all.

What are your thoughts?

[WhatsACloud]

I believe you raise a very valid concern. I'll try the approaches you have suggested. Thank you very much for your feedback!

[WhatsACloud]

Hi @DHowett! I found a HardReset and SoftReset function in AdaptDispatch.cpp which looks promising.

However, it is quite difficult to access the method and I need your advice on how to approach this problem.

I've created a function called HardReset in terminal.cpp which attempts to access the instantiated AdaptDispatch class within OutputStateMachineEngine within StateMachine in order to call this HardReset function from AdaptDispatch.cpp.

the code below outlines the nesting structure.

    // terminal.cpp
    auto dispatch = std::make_unique<AdaptDispatch>(*this, &renderer, _renderSettings, _terminalInput);
    auto engine = std::make_unique<OutputStateMachineEngine>(std::move(dispatch));
    _stateMachine = std::make_unique<StateMachine>(std::move(engine));

the problem is that StateMachine stores the engine as the class IStateMachineEngine rather than as the class OutputStateMachineEngine. Only OutputStateMachineEngine has the function Dispatch() which allows one to access the initialised AdaptDispatch class. This same issue also occurs for accessing the HardReset function inside AdaptDispatch.(Dispatch() returns ITermDispatch. HardReset is only present in AdaptDispatch, not ITermDispatch.)

I thought of using dynamic_cast to cast the IState to an OutputState initially, but I got this error message (see below), and seeing how dynamic_cast is not used anywhere in the project, I would assume it is a project design decision to stay away from it.

'dynamic_cast' used on polymorphic type 'Microsoft::Console::VirtualTerminal::IStateMachineEngine' with /GR-; unpredictable behavior may result

3 possible solutions to this are:

1: adding .Dispatch() to IStateMachineEngine, and adding .HardReset() to ITermDispatch

I did a bit more exploring, and noticed that the 2 areas where StateMachineEngine is used are [terminal.cpp] and [VtInputThread.cpp].

    // VtInputThread.cpp
    auto dispatch = std::make_unique<InteractDispatch>();
    auto engine = std::make_unique<InputStateMachineEngine>(std::move(dispatch), inheritCursor);
    _pInputStateMachine = std::make_unique<StateMachine>(std::move(engine));

It thus may be useful for both engine classes (Input and Output) to be able to access dispatch via a shared Dispatch function in IStateMachineEngine (both classes inherit from this base class). That would also partially solve the problem with accessing .Dispatch() from StateMachine in terminal.cpp.

As for accessing the .Dispatch() method from the engine, I noticed that the only class that inherits from ITermDispatch is AdaptDispatch, so it perhaps may be fine to either 1. shift HardReset ITermDispatch, or 2. change OutputStateMachineEngine to use AdaptDispatch instead of ITermDispatch.

2: making 2 separate StateMachine classes, one for Input and one for Output.

OutputStateMachine's .Engine() function would return a OutputStateMachineEngine instead of an IStateMachineEngine.
To reuse code we can make them inherit from a base StateMachine class.

3: ditching the API and just entering control sequences directly

self explanatory.

What do you think?