Windows 10 new console: try to avoid freezing the console while scraping
Fixes https://github.com/rprichard/winpty/issues/53
This commit is contained in:
parent
b8c2c41eb7
commit
fa6ba9c725
62
misc/FreezePerfTest.cc
Normal file
62
misc/FreezePerfTest.cc
Normal file
@ -0,0 +1,62 @@
|
||||
#include <windows.h>
|
||||
|
||||
#include "TestUtil.cc"
|
||||
|
||||
const int SC_CONSOLE_MARK = 0xFFF2;
|
||||
const int SC_CONSOLE_SELECT_ALL = 0xFFF5;
|
||||
|
||||
int main(int argc, char *argv[0]) {
|
||||
|
||||
if (argc != 2) {
|
||||
printf("Usage: %s (mark|selectall|read)\n", argv[0]);
|
||||
return 1;
|
||||
}
|
||||
|
||||
enum class Test { Mark, SelectAll, Read } test;
|
||||
if (!strcmp(argv[1], "mark")) {
|
||||
test = Test::Mark;
|
||||
} else if (!strcmp(argv[1], "selectall")) {
|
||||
test = Test::SelectAll;
|
||||
} else if (!strcmp(argv[1], "read")) {
|
||||
test = Test::Read;
|
||||
} else {
|
||||
printf("Invalid test: %s\n", argv[1]);
|
||||
return 1;
|
||||
}
|
||||
|
||||
HANDLE conout = GetStdHandle(STD_OUTPUT_HANDLE);
|
||||
TimeMeasurement tm;
|
||||
HWND hwnd = GetConsoleWindow();
|
||||
|
||||
setWindowPos(0, 0, 1, 1);
|
||||
setBufferSize(100, 3000);
|
||||
system("cls");
|
||||
setWindowPos(0, 2975, 100, 25);
|
||||
setCursorPos(0, 2999);
|
||||
|
||||
ShowWindow(hwnd, SW_HIDE);
|
||||
|
||||
for (int i = 0; i < 1000; ++i) {
|
||||
// CONSOLE_SCREEN_BUFFER_INFO info = {};
|
||||
// GetConsoleScreenBufferInfo(conout, &info);
|
||||
|
||||
if (test == Test::Mark) {
|
||||
SendMessage(hwnd, WM_SYSCOMMAND, SC_CONSOLE_MARK, 0);
|
||||
SendMessage(hwnd, WM_CHAR, 27, 0x00010001);
|
||||
} else if (test == Test::SelectAll) {
|
||||
SendMessage(hwnd, WM_SYSCOMMAND, SC_CONSOLE_SELECT_ALL, 0);
|
||||
SendMessage(hwnd, WM_CHAR, 27, 0x00010001);
|
||||
} else if (test == Test::Read) {
|
||||
static CHAR_INFO buffer[100 * 3000];
|
||||
const SMALL_RECT readRegion = {0, 0, 99, 2999};
|
||||
SMALL_RECT tmp = readRegion;
|
||||
BOOL ret = ReadConsoleOutput(conout, buffer, {100, 3000}, {0, 0}, &tmp);
|
||||
ASSERT(ret && !memcmp(&tmp, &readRegion, sizeof(tmp)));
|
||||
}
|
||||
}
|
||||
|
||||
ShowWindow(hwnd, SW_SHOW);
|
||||
|
||||
printf("elapsed: %f\n", tm.elapsed());
|
||||
return 0;
|
||||
}
|
27
misc/WindowsBugCrashReader.cc
Normal file
27
misc/WindowsBugCrashReader.cc
Normal file
@ -0,0 +1,27 @@
|
||||
// I noticed this on the ConEmu web site:
|
||||
//
|
||||
// https://social.msdn.microsoft.com/Forums/en-US/40c8e395-cca9-45c8-b9b8-2fbe6782ac2b/readconsoleoutput-cause-access-violation-writing-location-exception
|
||||
// https://conemu.github.io/en/MicrosoftBugs.html
|
||||
//
|
||||
// In Windows 7, 8, and 8.1, a ReadConsoleOutputW with an out-of-bounds read
|
||||
// region crashes the application. I have reproduced the problem on Windows 8
|
||||
// and 8.1, but not on Windows 7.
|
||||
//
|
||||
|
||||
#include <windows.h>
|
||||
|
||||
#include "TestUtil.cc"
|
||||
|
||||
int main() {
|
||||
setWindowPos(0, 0, 1, 1);
|
||||
setBufferSize(80, 25);
|
||||
setWindowPos(0, 0, 80, 25);
|
||||
|
||||
const HANDLE conout = openConout();
|
||||
static CHAR_INFO lineBuf[80];
|
||||
SMALL_RECT readRegion = { 0, 999, 79, 999 };
|
||||
const BOOL ret = ReadConsoleOutputW(conout, lineBuf, {80, 1}, {0, 0}, &readRegion);
|
||||
ASSERT(!ret && "ReadConsoleOutputW should have failed");
|
||||
|
||||
return 0;
|
||||
}
|
@ -524,7 +524,7 @@ void Agent::resizeWindow(const int cols, const int rows)
|
||||
trace("resizeWindow: invalid size: cols=%d,rows=%d", cols, rows);
|
||||
return;
|
||||
}
|
||||
Win32Console::FreezeGuard guard(m_console, true);
|
||||
Win32Console::FreezeGuard guard(m_console, m_console.frozen());
|
||||
const Coord newSize(cols, rows);
|
||||
ConsoleScreenBufferInfo info;
|
||||
m_primaryScraper->resizeWindow(*openPrimaryBuffer(), newSize, info);
|
||||
@ -536,7 +536,7 @@ void Agent::resizeWindow(const int cols, const int rows)
|
||||
|
||||
void Agent::scrapeBuffers()
|
||||
{
|
||||
Win32Console::FreezeGuard guard(m_console, true);
|
||||
Win32Console::FreezeGuard guard(m_console, m_console.frozen());
|
||||
ConsoleScreenBufferInfo info;
|
||||
m_primaryScraper->scrapeBuffer(*openPrimaryBuffer(), info);
|
||||
m_consoleInput->setMouseWindowRect(info.windowRect());
|
||||
|
@ -93,6 +93,7 @@ Scraper::~Scraper()
|
||||
{
|
||||
}
|
||||
|
||||
// Whether or not the agent is frozen on entry, it will be frozen on exit.
|
||||
void Scraper::resizeWindow(Win32ConsoleBuffer &buffer,
|
||||
Coord newSize,
|
||||
ConsoleScreenBufferInfo &finalInfoOut)
|
||||
@ -103,6 +104,7 @@ void Scraper::resizeWindow(Win32ConsoleBuffer &buffer,
|
||||
m_consoleBuffer = nullptr;
|
||||
}
|
||||
|
||||
// This function may freeze the agent, but it will not unfreeze it.
|
||||
void Scraper::scrapeBuffer(Win32ConsoleBuffer &buffer,
|
||||
ConsoleScreenBufferInfo &finalInfoOut)
|
||||
{
|
||||
@ -310,7 +312,18 @@ void Scraper::syncConsoleContentAndSize(
|
||||
bool forceResize,
|
||||
ConsoleScreenBufferInfo &finalInfoOut)
|
||||
{
|
||||
Win32Console::FreezeGuard guard(m_console, true);
|
||||
// We'll try to avoid freezing the console by reading large chunks (or
|
||||
// all!) of the screen buffer without otherwise attempting to synchronize
|
||||
// with the console application. We can only do this on Windows 10 and up
|
||||
// because:
|
||||
// - Prior to Windows 8, the size of a ReadConsoleOutputW call was limited
|
||||
// by the ~32KB RPC buffer.
|
||||
// - Prior to Windows 10, an out-of-range read region crashes the caller.
|
||||
// (See misc/WindowsBugCrashReader.cc.)
|
||||
//
|
||||
if (!m_console.isNewW10() || forceResize) {
|
||||
m_console.setFrozen(true);
|
||||
}
|
||||
|
||||
const ConsoleScreenBufferInfo info = m_consoleBuffer->bufferInfo();
|
||||
BOOL cursorVisible = true;
|
||||
@ -332,6 +345,7 @@ void Scraper::syncConsoleContentAndSize(
|
||||
// When we switch from direct->scrolling mode, make sure the console is
|
||||
// the right size.
|
||||
if (!m_directMode) {
|
||||
m_console.setFrozen(true);
|
||||
forceResize = true;
|
||||
}
|
||||
}
|
||||
@ -339,7 +353,14 @@ void Scraper::syncConsoleContentAndSize(
|
||||
if (m_directMode) {
|
||||
directScrapeOutput(info, cursorVisible);
|
||||
} else {
|
||||
scrollingScrapeOutput(info, cursorVisible);
|
||||
if (!m_console.frozen()) {
|
||||
if (!scrollingScrapeOutput(info, cursorVisible, true)) {
|
||||
m_console.setFrozen(true);
|
||||
}
|
||||
}
|
||||
if (m_console.frozen()) {
|
||||
scrollingScrapeOutput(info, cursorVisible, false);
|
||||
}
|
||||
}
|
||||
|
||||
if (forceResize) {
|
||||
@ -397,8 +418,9 @@ void Scraper::directScrapeOutput(const ConsoleScreenBufferInfo &info,
|
||||
}
|
||||
}
|
||||
|
||||
void Scraper::scrollingScrapeOutput(const ConsoleScreenBufferInfo &info,
|
||||
bool cursorVisible)
|
||||
bool Scraper::scrollingScrapeOutput(const ConsoleScreenBufferInfo &info,
|
||||
bool cursorVisible,
|
||||
bool tentative)
|
||||
{
|
||||
const Coord cursor = info.cursorPosition();
|
||||
const SmallRect windowRect = info.windowRect();
|
||||
@ -406,8 +428,13 @@ void Scraper::scrollingScrapeOutput(const ConsoleScreenBufferInfo &info,
|
||||
if (m_syncRow != -1) {
|
||||
// If a synchronizing marker was placed into the history, look for it
|
||||
// and adjust the scroll count.
|
||||
int markerRow = findSyncMarker();
|
||||
const int markerRow = findSyncMarker();
|
||||
if (markerRow == -1) {
|
||||
if (tentative) {
|
||||
// I *think* it's possible to keep going, but it's simple to
|
||||
// bail out.
|
||||
return false;
|
||||
}
|
||||
// Something has happened. Reset the terminal.
|
||||
trace("Sync marker has disappeared -- resetting the terminal"
|
||||
" (m_syncCounter=%u)",
|
||||
@ -422,6 +449,17 @@ void Scraper::scrollingScrapeOutput(const ConsoleScreenBufferInfo &info,
|
||||
}
|
||||
}
|
||||
|
||||
// Creating a new sync row requires clearing part of the console buffer, so
|
||||
// avoid doing it if there's already a sync row that's good enough.
|
||||
// TODO: replace hard-coded constants
|
||||
const int newSyncRow = static_cast<int>(windowRect.top()) - 200;
|
||||
const bool shouldCreateSyncRow = newSyncRow >= m_syncRow + 200;
|
||||
if (tentative && shouldCreateSyncRow) {
|
||||
// It's difficult even in principle to put down a new marker if the
|
||||
// console can scroll an arbitrarily amount while we're writing.
|
||||
return false;
|
||||
}
|
||||
|
||||
// Update the dirty line count:
|
||||
// - If the window has moved, the entire window is dirty.
|
||||
// - Everything up to the cursor is dirty.
|
||||
@ -432,6 +470,11 @@ void Scraper::scrollingScrapeOutput(const ConsoleScreenBufferInfo &info,
|
||||
// The window has moved down, presumably as a result of scrolling.
|
||||
markEntireWindowDirty(windowRect);
|
||||
} else if (windowRect.top() < m_dirtyWindowTop) {
|
||||
if (tentative) {
|
||||
// I *think* it's possible to keep going, but it's simple to
|
||||
// bail out.
|
||||
return false;
|
||||
}
|
||||
// The window has moved upward. This is generally not expected to
|
||||
// happen, but the CMD/PowerShell CLS command will move the window
|
||||
// to the top as part of clearing everything else in the console.
|
||||
@ -470,6 +513,32 @@ void Scraper::scrollingScrapeOutput(const ConsoleScreenBufferInfo &info,
|
||||
MAX_CONSOLE_WIDTH),
|
||||
stopReadLine - firstReadLine));
|
||||
|
||||
// If we're scraping the buffer without freezing it, we have to query the
|
||||
// buffer position data separately from the buffer content, so the two
|
||||
// could easily be out-of-sync. If they *are* out-of-sync, abort the
|
||||
// scrape operation and restart it frozen. (We may have updated the
|
||||
// dirty-line high-water-mark, but that should be OK.)
|
||||
if (tentative) {
|
||||
const auto infoCheck = m_consoleBuffer->bufferInfo();
|
||||
if (info.bufferSize() != infoCheck.bufferSize() ||
|
||||
info.windowRect() != infoCheck.windowRect() ||
|
||||
info.cursorPosition() != infoCheck.cursorPosition()) {
|
||||
return false;
|
||||
}
|
||||
if (m_syncRow != -1 && m_syncRow != findSyncMarker()) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
if (shouldCreateSyncRow) {
|
||||
ASSERT(!tentative);
|
||||
createSyncMarker(newSyncRow);
|
||||
}
|
||||
|
||||
// At this point, we're finished interacting (reading or writing) the
|
||||
// console, and we just need to convert our collected data into terminal
|
||||
// output.
|
||||
|
||||
scanForDirtyLines(windowRect);
|
||||
|
||||
// Note that it's possible for all the lines on the current window to
|
||||
@ -511,17 +580,11 @@ void Scraper::scrollingScrapeOutput(const ConsoleScreenBufferInfo &info,
|
||||
|
||||
m_scrapedLineCount = windowRect.top() + m_scrolledCount;
|
||||
|
||||
// Creating a new sync row requires clearing part of the console buffer, so
|
||||
// avoid doing it if there's already a sync row that's good enough.
|
||||
// TODO: replace hard-coded constants
|
||||
const int newSyncRow = static_cast<int>(windowRect.top()) - 200;
|
||||
if (newSyncRow >= 1 && newSyncRow >= m_syncRow + 200) {
|
||||
createSyncMarker(newSyncRow);
|
||||
}
|
||||
|
||||
if (cursorVisible) {
|
||||
m_terminal->showTerminalCursor(cursorColumn, cursorLine);
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
void Scraper::syncMarkerText(CHAR_INFO (&output)[SYNC_MARKER_LEN])
|
||||
|
@ -71,8 +71,9 @@ private:
|
||||
ConsoleScreenBufferInfo &finalInfoOut);
|
||||
void directScrapeOutput(const ConsoleScreenBufferInfo &info,
|
||||
bool cursorVisible);
|
||||
void scrollingScrapeOutput(const ConsoleScreenBufferInfo &info,
|
||||
bool cursorVisible);
|
||||
bool scrollingScrapeOutput(const ConsoleScreenBufferInfo &info,
|
||||
bool cursorVisible,
|
||||
bool tentative);
|
||||
void syncMarkerText(CHAR_INFO (&output)[SYNC_MARKER_LEN]);
|
||||
int findSyncMarker();
|
||||
void createSyncMarker(int row);
|
||||
|
Loading…
Reference in New Issue
Block a user