Skip to content

Commit

Permalink
Specify SW_HIDE and avoid creating a background desktop for Win7 and up
Browse files Browse the repository at this point in the history
The SW_HIDE change fixes part of a ConEmu<->winpty incompatibility.  If
WINPTY_SHOW_CONSOLE=1 is set, and ConEmu's "start cmd" flag is on, then
readFontTable can still run forever.

Avoiding a background desktop would fix a ConEmu<->winpty incompatibility
if not for the previous fix.  It fixes a clipboard issue and
thread-unsafety issue, but only on Win7 and up.  (More work is needed for
XP/Vista.)

See #58.
See #70.
  • Loading branch information
rprichard committed Apr 10, 2016
1 parent ef339dc commit edc9792
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 1 deletion.
54 changes: 53 additions & 1 deletion src/libwinpty/winpty.cc
Original file line number Diff line number Diff line change
Expand Up @@ -212,12 +212,44 @@ static bool shouldShowConsoleWindow()
return GetEnvironmentVariableA("WINPTY_SHOW_CONSOLE", buf, sizeof(buf)) > 0;
}

static bool shouldCreateBackgroundDesktop() {
// Prior to Windows 7, winpty's repeated selection-deselection loop
// prevented the user from interacting with their *visible* console
// windows, unless we placed the console onto a background desktop.
// The SetProcessWindowStation call interferes with the clipboard and
// isn't thread-safe, though[1]. The call should perhaps occur in a
// special agent subprocess. Spawning a process in a background desktop
// also breaks ConEmu, but marking the process SW_HIDE seems to correct
// that[2].
//
// Windows 7 moved a lot of console handling out of csrss.exe and into
// a per-console conhost.exe process, which may explain why it isn't
// affected.
//
// This is a somewhat risky change, so there are low-level flags to
// assist in debugging if there are issues.
//
// [1] https://github.com/rprichard/winpty/issues/58
// [2] https://github.com/rprichard/winpty/issues/70
bool ret = !shouldShowConsoleWindow() && !isAtLeastWindows7();
const bool force = hasDebugFlag("force_desktop");
const bool suppress = hasDebugFlag("no_desktop");
if (force && suppress) {
trace("error: Both the force_desktop and no_desktop flags are set");
} else if (force) {
ret = true;
} else if (suppress) {
ret = false;
}
return ret;
}

// Get a non-interactive window station for the agent.
// TODO: review security w.r.t. windowstation and desktop.
static BackgroundDesktop setupBackgroundDesktop()
{
BackgroundDesktop ret;
if (!shouldShowConsoleWindow()) {
if (shouldCreateBackgroundDesktop()) {
const HWINSTA originalStation = GetProcessWindowStation();
ret.station = CreateWindowStationW(NULL, 0, WINSTA_ALL_ACCESS, NULL);
if (ret.station != NULL) {
Expand All @@ -230,6 +262,8 @@ static BackgroundDesktop setupBackgroundDesktop()
assert(ret.desktop != NULL);
ret.desktopName =
getObjectName(ret.station) + L"\\" + getObjectName(ret.desktop);
trace("Created background desktop: %s",
utf8FromWide(ret.desktopName).c_str());
} else {
trace("CreateWindowStationW failed");
}
Expand Down Expand Up @@ -257,6 +291,20 @@ static std::wstring getDesktopFullName()
return getObjectName(station) + L"\\" + getObjectName(desktop);
}

static bool shouldSpecifyHideFlag() {
const bool force = hasDebugFlag("force_sw_hide");
const bool suppress = hasDebugFlag("no_sw_hide");
bool ret = !shouldShowConsoleWindow();
if (force && suppress) {
trace("error: Both the force_sw_hide and no_sw_hide flags are set");
} else if (force) {
ret = true;
} else if (suppress) {
ret = false;
}
return ret;
}

static void startAgentProcess(const BackgroundDesktop &desktop,
const std::wstring &controlPipeName,
const std::wstring &dataPipeName,
Expand All @@ -278,6 +326,10 @@ static void startAgentProcess(const BackgroundDesktop &desktop,
STARTUPINFOW sui = {};
sui.cb = sizeof(sui);
sui.lpDesktop = desktop.station == NULL ? NULL : desktopV.data();
if (shouldSpecifyHideFlag()) {
sui.dwFlags |= STARTF_USESHOWWINDOW;
sui.wShowWindow = SW_HIDE;
}
PROCESS_INFORMATION pi = {};
const BOOL success =
CreateProcessW(exePath.c_str(),
Expand Down
5 changes: 5 additions & 0 deletions src/shared/WindowsVersion.cc
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,11 @@ bool isAtLeastWindowsVista() {
return getWindowsVersion() >= Version(6, 0);
}

// Returns true for Windows 7 (or Windows Server 2008 R2) or newer.
bool isAtLeastWindows7() {
return getWindowsVersion() >= Version(6, 1);
}

// Returns true for Windows 8 (or Windows Server 2012) or newer.
bool isAtLeastWindows8() {
return getWindowsVersion() >= Version(6, 2);
Expand Down
1 change: 1 addition & 0 deletions src/shared/WindowsVersion.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#define WINPTY_SHARED_WINDOWS_VERSION_H

bool isAtLeastWindowsVista();
bool isAtLeastWindows7();
bool isAtLeastWindows8();
void dumpWindowsVersion();

Expand Down

0 comments on commit edc9792

Please sign in to comment.