MDEV-11499 mysqltest, Windows : improve diagnostics if server fails to shutdown

Create minidump when server fails to shutdown. If process is being
debugged, cause a debug break.

Moves some code which is part of safe_kill into mysys, as both safe_kill,
and mysqltest produce minidumps on different timeouts.

Small cleanup in wait_until_dead() - replace inefficient loop with a single
wait.

client/mysqltest.cc

@@ -5026,13 +5026,34 @@ int query_get_string(MYSQL* mysql, const char* query,
 }
 
 
+#ifdef _WIN32
+#define SIGKILL 9
+#include <my_minidump.h>
 static int my_kill(int pid, int sig)
 {
-  DBUG_PRINT("info", ("Killing server, pid: %d", pid));
-#ifdef _WIN32
-#define SIGKILL 9 /* ignored anyway, see below */
   HANDLE proc;
-  if ((proc= OpenProcess(SYNCHRONIZE|PROCESS_TERMINATE, FALSE, pid)) == NULL)
+  if (sig == SIGABRT)
+  {
+    /*
+     Create a minidump. If process is being debugged, debug break
+     Otherwise, terminate.
+    */
+    verbose_msg("Aborting %d",pid);
+    my_create_minidump(pid,TRUE);
+    proc= OpenProcess(PROCESS_ALL_ACCESS, FALSE, pid);
+    if(!proc)
+      return -1;
+    BOOL debugger_present;
+    if (CheckRemoteDebuggerPresent(proc,&debugger_present) && debugger_present)
+    {
+      if (DebugBreakProcess(proc))
+      {
+        CloseHandle(proc);
+        return 0;
+      }
+    }
+  }
+  else if ((proc= OpenProcess(SYNCHRONIZE|PROCESS_TERMINATE, FALSE, pid)) == NULL)
     return -1;
   if (sig == 0)
   {
@@ -5043,12 +5064,30 @@ static int my_kill(int pid, int sig)
   (void)TerminateProcess(proc, 201);
   CloseHandle(proc);
   return 1;
-#else
-  return kill(pid, sig);
-#endif
 }
 
 
+/* Wait until process is gone, with timeout */
+static int wait_until_dead(int pid, int timeout)
+{
+  HANDLE proc= OpenProcess(SYNCHRONIZE, FALSE, pid);
+  if (!proc)
+    return 0; /* already dead */
+  DBUG_ASSERT(timeout >= 0);
+  DBUG_ASSERT(timeout <= UINT_MAX/1000);
+  DWORD wait_result= WaitForSingleObject(proc, (DWORD)timeout*1000);
+  CloseHandle(proc);
+  return (int)wait_result;
+}
+
+#else /* !_WIN32 */
+
+
+static int my_kill(int pid, int sig)
+{
+  DBUG_PRINT("info", ("Killing server, pid: %d", pid));
+  return kill(pid, sig);
+}
 
 /*
   Shutdown the server of current connection and
@@ -5083,6 +5122,7 @@ static int wait_until_dead(int pid, int timeout)
   }
   DBUG_RETURN(1);                               // Did not die
 }
+#endif /* _WIN32 */
 
 
 void do_shutdown_server(struct st_command *command)

include/my_minidump.h

@@ -0,0 +1,25 @@
+/* Copyright (c) 2021, MariaDB Corporation
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; version 2 of the License.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1335  USA */
+
+#include <windows.h>
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+BOOL my_create_minidump(DWORD pid, BOOL verbose);
+
+#ifdef __cplusplus
+}
+#endif

mysql-test/lib/My/SafeProcess/CMakeLists.txt

@@ -17,7 +17,8 @@
 IF (WIN32)
   ADD_EXECUTABLE(my_safe_process safe_process_win.cc)
   ADD_EXECUTABLE(my_safe_kill safe_kill_win.cc)
-  TARGET_LINK_LIBRARIES(my_safe_kill dbghelp psapi)
+  TARGET_INCLUDE_DIRECTORIES(my_safe_kill PRIVATE ${CMAKE_SOURCE_DIR}/include)
+  TARGET_LINK_LIBRARIES(my_safe_kill mysys psapi)
 ELSE()
   ADD_EXECUTABLE(my_safe_process safe_process.cc)
 ENDIF()

mysql-test/lib/My/SafeProcess/safe_kill_win.cc

@@ -26,19 +26,7 @@
 #include <signal.h>
 #include <stdlib.h>
 #include <psapi.h>
-
+#include <my_minidump.h>
-#ifdef _MSC_VER
-/* Silence warning in OS header dbghelp.h */
-#pragma warning(push)
-#pragma warning(disable : 4091)
-#endif
-
-#include <dbghelp.h>
-
-#ifdef _MSC_VER
-/* Silence warning in OS header dbghelp.h */
-#pragma warning(pop)
-#endif
 
 #include <tlhelp32.h>
 #include <vector>
@@ -64,106 +52,13 @@ static std::vector<DWORD> find_children(DWORD pid)
   return children;
 }
 
-void dump_single_process(DWORD pid)
-{
-  HANDLE file = 0;
-  HANDLE process= 0;
-  DWORD size= MAX_PATH;
-  char path[MAX_PATH];
-  char working_dir[MAX_PATH];
-  char tmpname[MAX_PATH];
-  char *filename= 0;
-
-  process= OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, pid);
-  if (!process)
-  {
-    fprintf(stderr, "safe_kill : cannot open process pid=%lu to create dump, last error %lu\n",
-      pid, GetLastError());
-    goto exit;
-  }
-
-  if (QueryFullProcessImageName(process, 0, path, &size) == 0)
-  {
-    fprintf(stderr, "safe_kill : cannot read process path for pid %lu, last error %lu\n",
-      pid, GetLastError());
-    goto exit;
-  }
-
-  filename= strrchr(path, '\\');
-  if (filename)
-  {
-    filename++;
-    // We are not interested in dump of some proceses (my_safe_process.exe,cmd.exe)
-    // since they are only used to start up other programs.
-    // We're interested however in their children;
-    const char *exclude_programs[] = {"my_safe_process.exe","cmd.exe", 0};
-    for(size_t i=0; exclude_programs[i]; i++)
-      if (_stricmp(filename, exclude_programs[i]) == 0)
-        goto exit;
-  }
-  else
-    filename= path;
-
-  // Add .dmp extension
-  char *p;
-  if ((p= strrchr(filename, '.')) == 0)
-    p= filename + strlen(filename);
-
-  strncpy(p, ".dmp", path + MAX_PATH - p);
-
-  // Íf file with this name exist, generate unique name with .dmp extension
-  if (GetFileAttributes(filename) != INVALID_FILE_ATTRIBUTES)
-  {
-    if (!GetTempFileName(".", filename, 0, tmpname))
-    {
-      fprintf(stderr, "GetTempFileName failed, last error %lu", GetLastError());
-      goto exit;
-    }
-    strncat_s(tmpname, ".dmp", sizeof(tmpname));
-    filename= tmpname;
-  }
-
-  
-  if (!GetCurrentDirectory(MAX_PATH, working_dir))
-  {
-    fprintf(stderr, "GetCurrentDirectory failed, last error %lu", GetLastError());
-    goto exit;
-  }
-
-  file= CreateFile(filename, GENERIC_READ | GENERIC_WRITE,
-    0, 0, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, 0);
-
-  if (file == INVALID_HANDLE_VALUE)
-  {
-    fprintf(stderr, "safe_kill : CreateFile() failed for file %s, working dir %s, last error = %lu\n",
-      filename, working_dir, GetLastError());
-    goto exit;
-  }
-
-  if (!MiniDumpWriteDump(process, pid, file, MiniDumpNormal, 0, 0, 0))
-  {
-    fprintf(stderr, "Failed to write minidump to %s, working dir %s, last error %lu\n",
-      filename, working_dir, GetLastError());
-    goto exit;
-  }
-
-  fprintf(stderr, "Minidump written to %s, directory %s\n", filename, working_dir);
-
-exit:
-  if (process != 0 && process != INVALID_HANDLE_VALUE)
-    CloseHandle(process);
-
-  if (file != 0 && file != INVALID_HANDLE_VALUE)
-    CloseHandle(file);
-}
-
 
 static int create_dump(DWORD pid, int recursion_depth= 5)
 {
   if (recursion_depth < 0)
     return 0;
 
-  dump_single_process(pid);
+  my_create_minidump(pid, TRUE);
   std::vector<DWORD> children= find_children(pid);
   for(size_t i=0; i < children.size(); i++)
     create_dump(children[i], recursion_depth -1);

mysys/CMakeLists.txt

@@ -48,7 +48,7 @@ SET(MYSYS_SOURCES  array.c charset-def.c charset.c checksum.c my_default.c
                                 file_logger.c my_dlerror.c)
 
 IF (WIN32)
- SET (MYSYS_SOURCES ${MYSYS_SOURCES} my_winthread.c my_wincond.c my_winerr.c my_winfile.c my_windac.c my_conio.c)
+ SET (MYSYS_SOURCES ${MYSYS_SOURCES} my_winthread.c my_wincond.c my_winerr.c my_winfile.c my_windac.c my_conio.c my_minidump.cc)
 ENDIF()
 
 IF(UNIX)
@@ -83,7 +83,7 @@ IF(HAVE_BFD_H)
 ENDIF(HAVE_BFD_H)
 
 IF (WIN32)
-  TARGET_LINK_LIBRARIES(mysys IPHLPAPI)  
+  TARGET_LINK_LIBRARIES(mysys iphlpapi dbghelp)
 ENDIF(WIN32)
 
 # Need explicit pthread for gcc -fsanitize=address

mysys/my_minidump.cc

@@ -0,0 +1,115 @@
+/* Copyright (c) 2021, MariaDB Corporation
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; version 2 of the License.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1335  USA */
+
+#include <windows.h>
+#include <dbghelp.h>
+#include <stdio.h>
+#include <my_minidump.h>
+
+#define VERBOSE(fmt,...) \
+  if (verbose)  { fprintf(stderr, "my_create_minidump : " fmt,__VA_ARGS__); }
+
+extern "C" BOOL my_create_minidump(DWORD pid, BOOL verbose)
+{
+  HANDLE file = 0;
+  HANDLE process= 0;
+  DWORD size= MAX_PATH;
+  char path[MAX_PATH];
+  char working_dir[MAX_PATH];
+  char tmpname[MAX_PATH];
+  char *filename= 0;
+  bool ret= FALSE;
+  process= OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, pid);
+  if (!process)
+  {
+    VERBOSE("cannot open process pid=%lu to create dump, last error %lu\n",
+      pid, GetLastError());
+    goto exit;
+  }
+
+  if (QueryFullProcessImageName(process, 0, path, &size) == 0)
+  {
+    VERBOSE("cannot read process path for pid %lu, last error %lu\n",
+      pid, GetLastError());
+    goto exit;
+  }
+
+  filename= strrchr(path, '\\');
+  if (filename)
+  {
+    filename++;
+    // We are not interested in dump of some proceses (my_safe_process.exe,cmd.exe)
+    // since they are only used to start up other programs.
+    // We're interested however in their children;
+    const char *exclude_programs[] = {"my_safe_process.exe","cmd.exe", 0};
+    for(size_t i=0; exclude_programs[i]; i++)
+      if (_stricmp(filename, exclude_programs[i]) == 0)
+        goto exit;
+  }
+  else
+    filename= path;
+
+  // Add .dmp extension
+  char *p;
+  if ((p= strrchr(filename, '.')) == 0)
+    p= filename + strlen(filename);
+
+  strncpy(p, ".dmp", path + MAX_PATH - p);
+
+  // Íf file with this name exist, generate unique name with .dmp extension
+  if (GetFileAttributes(filename) != INVALID_FILE_ATTRIBUTES)
+  {
+    if (!GetTempFileName(".", filename, 0, tmpname))
+    {
+      fprintf(stderr, "GetTempFileName failed, last error %lu", GetLastError());
+      goto exit;
+    }
+    strncat_s(tmpname, ".dmp", sizeof(tmpname));
+    filename= tmpname;
+  }
+
+  if (!GetCurrentDirectory(MAX_PATH, working_dir))
+  {
+    VERBOSE("GetCurrentDirectory failed, last error %lu", GetLastError());
+    goto exit;
+  }
+
+  file= CreateFile(filename, GENERIC_READ | GENERIC_WRITE,
+    0, 0, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, 0);
+
+  if (file == INVALID_HANDLE_VALUE)
+  {
+    VERBOSE("CreateFile() failed for file %s, working dir %s, last error = %lu\n",
+      filename, working_dir, GetLastError());
+    goto exit;
+  }
+
+  if (!MiniDumpWriteDump(process, pid, file, MiniDumpNormal, 0, 0, 0))
+  {
+    VERBOSE("Failed to write minidump to %s, working dir %s, last error %lu\n",
+      filename, working_dir, GetLastError());
+    goto exit;
+  }
+
+  VERBOSE("Minidump written to %s, directory %s\n", filename, working_dir);
+  ret= TRUE;
+exit:
+  if (process != 0 && process != INVALID_HANDLE_VALUE)
+    CloseHandle(process);
+
+  if (file != 0 && file != INVALID_HANDLE_VALUE)
+    CloseHandle(file);
+  return ret;
+}