From 6c65edb0e442a5cbdfcdd70e8af81b857f0fc123 Mon Sep 17 00:00:00 2001 From: Ryan Prichard Date: Mon, 29 Feb 2016 04:06:50 -0600 Subject: [PATCH] Make WinptyAssert.h behave more sensibly, both inside and outside the agent * In agent: if the console is frozen, then calling abort hangs forever, because abort tries to write to stderr, which blocks. Instead, use WM_CLOSE to close the console window. If that doesn't work after waiting a while, then exit. We need to do the same thing doing normal agent shutdown, so factor out an agentShutdown function. It's a bit unhappy living in WinptyAssert.cc, but it works. * Outside agent: trace an assertion failure message, then delegate to the ordinary assert() function. If that's somehow turned off, then call abort(). --- src/agent/Agent.cc | 5 +++-- src/agent/Win32Console.cc | 7 ------- src/agent/Win32Console.h | 1 - src/agent/main.cc | 4 ++++ src/agent/subdir.mk | 2 +- src/shared/WinptyAssert.cc | 38 ++++++++++++++++++++++++++-------- src/shared/WinptyAssert.h | 42 +++++++++++++++++++++++++++++++++++--- src/winpty.gyp | 3 +++ 8 files changed, 79 insertions(+), 23 deletions(-) diff --git a/src/agent/Agent.cc b/src/agent/Agent.cc index 84e21a7..f4f5c6c 100644 --- a/src/agent/Agent.cc +++ b/src/agent/Agent.cc @@ -180,9 +180,10 @@ Agent::Agent(LPCWSTR controlPipeName, Agent::~Agent() { trace("Agent exiting..."); - m_console->postCloseMessage(); - if (m_childProcess != NULL) + agentShutdown(); + if (m_childProcess != NULL) { CloseHandle(m_childProcess); + } delete m_console; delete m_terminal; delete m_consoleInput; diff --git a/src/agent/Win32Console.cc b/src/agent/Win32Console.cc index b9c7f8e..dc0a435 100644 --- a/src/agent/Win32Console.cc +++ b/src/agent/Win32Console.cc @@ -58,13 +58,6 @@ HWND Win32Console::hwnd() return GetConsoleWindow(); } -void Win32Console::postCloseMessage() -{ - HWND h = hwnd(); - if (h != NULL) - PostMessage(h, WM_CLOSE, 0, 0); -} - void Win32Console::clearLines( int row, int count, diff --git a/src/agent/Win32Console.h b/src/agent/Win32Console.h index c1549de..029e192 100644 --- a/src/agent/Win32Console.h +++ b/src/agent/Win32Console.h @@ -51,7 +51,6 @@ public: HANDLE conin(); HANDLE conout(); HWND hwnd(); - void postCloseMessage(); void clearLines(int row, int count, const ConsoleScreenBufferInfo &info); void clearAllLines(const ConsoleScreenBufferInfo &info); diff --git a/src/agent/main.cc b/src/agent/main.cc index 3611de9..c61408f 100644 --- a/src/agent/main.cc +++ b/src/agent/main.cc @@ -76,4 +76,8 @@ int main(int argc, char *argv[]) atoi(argv[3]), atoi(argv[4])); agent.run(); + + // The Agent destructor shouldn't return, but if it does, exit + // unsuccessfully. + return 1; } diff --git a/src/agent/subdir.mk b/src/agent/subdir.mk index 257b3f8..d186c30 100644 --- a/src/agent/subdir.mk +++ b/src/agent/subdir.mk @@ -20,7 +20,7 @@ ALL_TARGETS += build/winpty-agent.exe -$(eval $(call def_mingw_target,agent,)) +$(eval $(call def_mingw_target,agent,-DWINPTY_AGENT_ASSERT)) AGENT_OBJECTS = \ build/agent/agent/Agent.o \ diff --git a/src/shared/WinptyAssert.cc b/src/shared/WinptyAssert.cc index b95a821..1ff0de4 100755 --- a/src/shared/WinptyAssert.cc +++ b/src/shared/WinptyAssert.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011-2012 Ryan Prichard +// Copyright (c) 2011-2016 Ryan Prichard // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to @@ -19,17 +19,37 @@ // IN THE SOFTWARE. #include "WinptyAssert.h" -#include "DebugClient.h" + +#include #include -// Calling the standard assert() function does not work in the agent because -// the error message would be printed to the console, and the only way the -// user can see the console is via a working agent! This custom assert -// function instead sends the message to the DebugServer. +#include "DebugClient.h" -void assertFail(const char *file, int line, const char *cond) -{ +void assertTrace(const char *file, int line, const char *cond) { trace("Assertion failed: %s, file %s, line %d", cond, file, line); - abort(); } + +#ifdef WINPTY_AGENT_ASSERT + +void agentShutdown() { + HWND hwnd = GetConsoleWindow(); + if (hwnd != NULL) { + PostMessage(hwnd, WM_CLOSE, 0, 0); + Sleep(30000); + trace("Agent shutdown: WM_CLOSE did not end agent process"); + } else { + trace("Agent shutdown: GetConsoleWindow() is NULL"); + } + // abort() prints a message to the console, and if it is frozen, then the + // process would hang, so instead use exit(). (We shouldn't ever get here, + // though, because the WM_CLOSE message should have ended this process.) + exit(1); +} + +void agentAssertFail(const char *file, int line, const char *cond) { + assertTrace(file, line, cond); + agentShutdown(); +} + +#endif diff --git a/src/shared/WinptyAssert.h b/src/shared/WinptyAssert.h index 327d1ac..b2b8b5e 100755 --- a/src/shared/WinptyAssert.h +++ b/src/shared/WinptyAssert.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011-2012 Ryan Prichard +// Copyright (c) 2011-2016 Ryan Prichard // // Permission is hereby granted, free of charge, to any person obtaining a copy // of this software and associated documentation files (the "Software"), to @@ -21,8 +21,44 @@ #ifndef WINPTY_ASSERT_H #define WINPTY_ASSERT_H -#define ASSERT(x) do { if (!(x)) assertFail(__FILE__, __LINE__, #x); } while(0) +#ifdef WINPTY_AGENT_ASSERT -void assertFail(const char *file, int line, const char *cond); +void agentShutdown(); +void agentAssertFail(const char *file, int line, const char *cond); + +// Calling the standard assert() function does not work in the agent because +// the error message would be printed to the console, and the only way the +// user can see the console is via a working agent! Moreover, the console may +// be frozen, so attempting to write to it would block forever. This custom +// assert function instead sends the message to the DebugServer, then attempts +// to close the console, then quietly exits. +#define ASSERT(cond) \ + do { \ + if (!(cond)) { \ + agentAssertFail(__FILE__, __LINE__, #cond); \ + } \ + } while(0) + +#else + +void assertTrace(const char *file, int line, const char *cond); + +// In the other targets, log the assert failure to the debugserver, then fail +// using the ordinary assert mechanism. In case assert is compiled out, fail +// using abort. The amount of code inlined is unfortunate, but asserts aren't +// used much outside the agent. +#include +#include +#define ASSERT_CONDITION(cond) (false && (cond)) +#define ASSERT(cond) \ + do { \ + if (!(cond)) { \ + assertTrace(__FILE__, __LINE__, #cond); \ + assert(ASSERT_CONDITION(#cond)); \ + abort(); \ + } \ + } while(0) + +#endif #endif // WINPTY_ASSERT_H diff --git a/src/winpty.gyp b/src/winpty.gyp index b1de6ed..f51d959 100644 --- a/src/winpty.gyp +++ b/src/winpty.gyp @@ -34,6 +34,9 @@ 'include_dirs' : [ 'include', ], + 'defines' : [ + 'WINPTY_AGENT_ASSERT', + ], 'libraries' : [ '-luser32', ],