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

@@ -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;

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,

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);
 

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;
 }

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 \

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 <windows.h>
 #include <stdlib.h>
 
-// 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

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 <assert.h>
+#include <stdlib.h>
+#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

src/winpty.gyp

@@ -34,6 +34,9 @@
             'include_dirs' : [
                 'include',
             ],
+            'defines' : [
+                'WINPTY_AGENT_ASSERT',
+            ],
             'libraries' : [
                 '-luser32',
             ],