Low reliability of VT replies

[alabuzhev]

Windows Terminal version

Latest source

Windows build number

10.0.19045.4780

Other Software

No

Steps to reproduce

I want to query some terminal state, in particular the latest and greatest palette (#17729).
To do so, I build a bunch of those "\x1b]4;N;?\x1b\\", send them in one go and wait for the reply.
Since I want to support not only the latest nightlies and do not want the older versions to deadlock on unknown queries, I prepend and append another query, the one that even prehistoric versions understand: "\x1b[c".
If there is something between those DA replies, it must be the one I need.
If there are only two DA replies, then it is probably not supported.

This approach works flawlessly in OpenConsole.
In WT, however, not so much: way, way too often random parts of the reply are simply missing.

1. Compile the code:

Code

#ifndef _CRT_SECURE_NO_WARNINGS
#define _CRT_SECURE_NO_WARNINGS
#endif

#ifndef _UNICODE
#define _UNICODE
#endif

#ifndef UNICODE
#define UNICODE
#endif

#include <algorithm>
#include <format>
#include <exception>
#include <iostream>
#include <optional>
#include <ranges>
#include <string>
#include <string_view>

#include <string.h>

#include <windows.h>

using namespace std::literals;

auto win32_error()
{
	return std::runtime_error(std::format("Error 0x{:08X}, look it up"sv, GetLastError()));
};

auto invalid_response()
{
	return std::runtime_error("Invalid response");
}

auto invalid_response(const size_t Index)
{
	return std::runtime_error(std::format("Invalid response at #{}"sv, Index));
}

auto printable(const std::wstring_view Str)
{
	std::wstring Printable;
	std::ranges::transform(Str, std::back_inserter(Printable), [](wchar_t Char){ return Char < L' ' ? Char + L'\u2400' : Char; });
	return Printable;
}

void write(const std::wstring_view Str)
{
	DWORD Written;
	if (!WriteConsole(GetStdHandle(STD_OUTPUT_HANDLE), Str.data(), static_cast<DWORD>(Str.size()), &Written, {}))
		throw win32_error();
}

void write(const std::string_view Str)
{
	write(std::wstring(Str.begin(), Str.end()));
}

std::wstring query(const std::wstring& Command)
{
	const auto Dummy = L"\x1b[c"s;

	write(Dummy + Command + Dummy);

	// Sleep(1);

	std::wstring Response;

	std::optional<size_t>
		FirstTokenPrefixPos,
		FirstTokenSuffixPos,
		SecondTokenPrefixPos,
		SecondTokenSuffixPos;

	const auto
		TokenPrefix = L"\x1b[?"sv,
		TokenSuffix = L"c"sv;

	while (!SecondTokenSuffixPos)
	{
		wchar_t ResponseBuffer[8192];
		DWORD ResponseSize;

		if (!ReadConsole(GetStdHandle(STD_INPUT_HANDLE), ResponseBuffer, ARRAYSIZE(ResponseBuffer),  &ResponseSize, {}))
			throw win32_error();

		Response.append(ResponseBuffer, ResponseSize);

		if (!FirstTokenPrefixPos)
			if (const auto Pos = Response.find(TokenPrefix); Pos != Response.npos)
				FirstTokenPrefixPos = Pos;

		if (FirstTokenPrefixPos && !FirstTokenSuffixPos)
			if (const auto Pos = Response.find(TokenSuffix, *FirstTokenPrefixPos + TokenPrefix.size()); Pos != Response.npos)
				FirstTokenSuffixPos = Pos;

		if (FirstTokenSuffixPos && !SecondTokenPrefixPos)
			if (const auto Pos = Response.find(TokenPrefix, *FirstTokenSuffixPos + TokenSuffix.size()); Pos != Response.npos)
				SecondTokenPrefixPos = Pos;

		if (SecondTokenPrefixPos && !SecondTokenSuffixPos)
			if (const auto Pos = Response.find(TokenSuffix, *SecondTokenPrefixPos + TokenPrefix.size()); Pos != Response.npos)
				SecondTokenSuffixPos = Pos;
	}

	Response.resize(*SecondTokenPrefixPos);
	Response.erase(0, *FirstTokenSuffixPos + TokenSuffix.size());

	if (Response.empty())
		throw std::runtime_error("Query is not supported"s);

	return Response;
}

void set_modes()
{
	const auto
		In = GetStdHandle(STD_INPUT_HANDLE),
		Out = GetStdHandle(STD_OUTPUT_HANDLE);

	DWORD InMode;
	if (!GetConsoleMode(In, &InMode))
		throw win32_error();

	if (!SetConsoleMode(In, (InMode & ~(ENABLE_LINE_INPUT | ENABLE_ECHO_INPUT | ENABLE_QUICK_EDIT_MODE)) | ENABLE_MOUSE_INPUT | ENABLE_EXTENDED_FLAGS | ENABLE_VIRTUAL_TERMINAL_INPUT))
		throw win32_error();

	DWORD OutMode;
	if (!GetConsoleMode(Out, &OutMode))
		throw win32_error();

	if (!SetConsoleMode(Out, OutMode | ENABLE_PROCESSED_OUTPUT | ENABLE_VIRTUAL_TERMINAL_PROCESSING))
		throw win32_error();
}

int main()
{
	try
	{
		set_modes();

		std::wstring Str;

		for (size_t i = 0; i != 256; ++i)
			std::format_to(std::back_inserter(Str), L"\x1b]4;{};?\x1b\\"sv, i);

		for (;;)
		{
			const auto ReplyData = query(Str);

			std::wstring_view Reply(ReplyData);

			if (!Reply.ends_with(L"\\"sv))
				throw invalid_response();

			Reply.remove_suffix(1);

			size_t Index = 0;

			for (const auto& Part: std::views::split(Reply, L"\\"sv))
			{
				std::wstring_view ColorStr(Part.begin(), Part.end());

				try
				{
					if (!ColorStr.starts_with(L"\x1b]4;"sv))
						throw invalid_response(Index);

					ColorStr.remove_prefix(L"\x1b]4;"sv.size());

					wchar_t* Ptr;
					const auto ReportedIndex = std::wcstol(ColorStr.data(), &Ptr, 10);

					if (Ptr == ColorStr.data())
						throw invalid_response(Index);

					if (ReportedIndex != Index)
						throw invalid_response(Index);

					ColorStr.remove_prefix(Ptr - ColorStr.data());

					const auto End = ColorStr.find(L"\x1b"sv);
					if (End == ColorStr.npos)
						throw invalid_response(Index);

					ColorStr.remove_suffix(ColorStr.size() - End);

					if (!ColorStr.starts_with(L";rgb:"sv))
						throw invalid_response(Index);

					ColorStr.remove_prefix(L";rgb:"sv.size());

					if (ColorStr.size() != L"ffff/ffff/ffff"sv.size())
						throw invalid_response(Index);
				}
				catch (const std::runtime_error& e)
				{
					Beep(500, 200);

					write(e.what());
					write(L": "sv);
					write(printable(ColorStr));
					write(L"\n"sv);
				}

				++Index;
			}

			std::wcout << L"Everything is OK"sv << std::endl;

		}
	}
	catch (const std::exception& e)
	{
		std::cerr << e.what() << std::endl;
	}
}

2. Run it in WT.

Expected Behavior

The program should print "Everything is OK" in a loop indefinitely and nothing else:

Everything is OK
Everything is OK
Everything is OK
Everything is OK
...and so on.

Actual Behavior

The program prints "Everything is OK" a few times, then it detects errors due to missing bits in the reply, e.g.

Everything is OK
Everything is OK
Everything is OK
Everything is OK
Invalid response at #234: c/1c1c␛
Everything is OK

Eventually it deadlocks in ReadConsole.

Adding a delay between sending the query and reading the reply improves the situation dramatically, but still not to 100% and it does not feel like the right thing to do: fixing issues with Sleep(N) never works in the long term.

Splitting it to smaller queries also helps, but again, it is guesswork.
Overall it is about 2700 characters to send and about 7000 to receive. It does not look like something humongous by modern standards.

Moving the mouse or pressing keyboard keys noticeably increases the error rate. Again, only in WT.

[lhecker]

I believe this is caused by #16224. Since the terminal lock isn't held anymore, any thread can write into the input pipe concurrently. In our case that's the VT parser thread sending VT responses and the UI thread sending cursor messages.

[alabuzhev]

Thanks.

and the UI thread sending cursor messages

Just to confirm: bad replies happen even when there are no cursor messages (ENABLE_MOUSE_INPUT is not set) or any other input.

Also, I don't know if it's related or not, but every time I terminate the repro with ^C the shell crashes:

I thought maybe I've built WT wrong or something, but the fact that it fails in ReadKey is suspicious.

[lhecker]

Just to confirm: bad replies happen even when there are no cursor messages (ENABLE_MOUSE_INPUT is not set) or any other input.

Aside from mouse messages and keyboard input, we also send focus events to ConPTY from the UI thread.

Also, I don't know if it's related or not, but every time I terminate the repro with ^C the shell crashes: [...]

Yeah, I noticed that as well. I believe it's dotnet/runtime#88697 which points to our issue #15859. I've set it to be fixed in v1.23 now. The weirdest thing is that this issue doesn't occur anymore after my PR. (It does still break PSReadLine though, because ENABLE_MOUSE_INPUT doesn't get disabled on exit.)

[alabuzhev]

Thanks for such a quick fix!