MDEV-21062 Buildbot, Windows - sporadically missing lines from mtr's "exec"

Provide own version of popen/pclose, in attempt to workaround
sporadic erratic behavior of UCRT's one.

client/mysql_upgrade.c

@@ -383,7 +383,7 @@ static int run_command(char* cmd,
   if (opt_verbose >= 4)
     puts(cmd);
 
-  if (!(res_file= popen(cmd, "r")))
+  if (!(res_file= my_popen(cmd, IF_WIN("rt","r"))))
     die("popen(\"%s\", \"r\") failed", cmd);
 
   while (fgets(buf, sizeof(buf), res_file))
@@ -401,7 +401,7 @@ static int run_command(char* cmd,
     }
   }
 
-  error= pclose(res_file);
+  error= my_pclose(res_file);
   return WEXITSTATUS(error);
 }
 

client/mysqltest.cc

@@ -872,14 +872,6 @@ static char *my_fgets(char * s, int n, FILE * stream, int *len)
   return buf;
 }
 
-/*
-  Wrapper for popen().
-*/
-static FILE* my_popen(const char *cmd, const char *mode)
-{
-  return popen(cmd, mode);
-}
-
 #ifdef EMBEDDED_LIBRARY
 
 #define EMB_SEND_QUERY 1
@@ -1854,7 +1846,7 @@ static int run_command(char* cmd,
     }
   }
 
-  error= pclose(res_file);
+  error= my_pclose(res_file);
   DBUG_RETURN(WEXITSTATUS(error));
 }
 
@@ -3440,7 +3432,7 @@ void do_exec(struct st_command *command)
   {
     replace_dynstr_append_mem(ds_result, buf, len);
   }
-  error= pclose(res_file);
+  error= my_pclose(res_file);
 
   if (display_result_sorted)
   {
@@ -4670,7 +4662,7 @@ void do_perl(struct st_command *command)
         replace_dynstr_append_mem(&ds_res, buf, len);
       }
     }
-    error= pclose(res_file);
+    error= my_pclose(res_file);
 
     /* Remove the temporary file, but keep it if perl failed */
     if (!error)

extra/perror.c

@@ -199,19 +199,22 @@ int get_ER_error_msg(uint code, const char **name_ptr, const char **msg_ptr)
   return 0;
 }
 
-#if defined(__WIN__)
+#if defined(_WIN32)
 static my_bool print_win_error_msg(DWORD error, my_bool verbose)
 {
-  LPTSTR s;
+  char *s;
-  if (FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER |
+  if (FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER |
                     FORMAT_MESSAGE_FROM_SYSTEM,
-                    NULL, error, 0, (LPTSTR)&s, 0,
+                    NULL, error, 0, (char *)&s, 0,
                     NULL))
   {
+    char* end = s + strlen(s) - 1;
+    while (end > s && (*end == '\r' || *end == '\n'))
+      *end-- = 0;
     if (verbose)
-      printf("Win32 error code %lu: %s", error, s);
+      printf("Win32 error code %lu: %s\n", error, s);
     else
-      puts(s);
+      printf("%s\n",s);
     LocalFree(s);
     return 0;
   }
@@ -259,7 +262,7 @@ int main(int argc,char *argv[])
   const char *msg;
   const char *name;
   char *unknown_error = 0;
-#if defined(__WIN__)
+#if defined(_WIN32)
   my_bool skip_win_message= 0;
 #endif
   MY_INIT(argv[0]);
@@ -350,17 +353,17 @@ int main(int argc,char *argv[])
       }
       if (!found)
       {
-#if defined(__WIN__)
+#if defined(_WIN32)
         if (!(skip_win_message= !print_win_error_msg((DWORD)code, verbose)))
         {
 #endif
           fprintf(stderr,"Illegal error code: %d\n",code);
           error=1;
-#if defined(__WIN__)
+#if defined(_WIN32)
         }
 #endif
       }
-#if defined(__WIN__)
+#if defined(_WIN32)
       if (!skip_win_message)
         print_win_error_msg((DWORD)code, verbose);
 #endif

include/my_sys.h

@@ -994,6 +994,16 @@ void *my_mmap(void *, size_t, int, int, int, my_off_t);
 int my_munmap(void *, size_t);
 #endif
 
+#ifdef _WIN32
+extern FILE* my_win_popen(const char*, const char*);
+extern int my_win_pclose(FILE*);
+#define my_popen(A,B) my_win_popen(A,B)
+#define my_pclose(A) my_win_pclose(A)
+#else
+#define my_popen(A,B) popen(A,B)
+#define my_pclose(A) pclose(A)
+#endif
+
 /* my_getpagesize */
 #ifdef HAVE_GETPAGESIZE
 #define my_getpagesize()        getpagesize()

mysys/CMakeLists.txt

@@ -48,7 +48,14 @@ 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_win_popen.cc)
 ENDIF()
 
 IF(UNIX)

mysys/my_win_popen.cc

@@ -0,0 +1,170 @@
+/* 2019, 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-1301  USA */
+
+/*
+ Replacement of the buggy implementations of popen in Windows CRT
+*/
+#include <windows.h>
+#include <fcntl.h>
+#include <io.h>
+#include <mutex>
+#include <stdlib.h>
+#include <unordered_map>
+
+enum
+{
+  REDIRECT_STDIN= 'w',
+  REDIRECT_STDOUT= 'r'
+};
+
+/** Map from FILE* returned by popen() to corresponding process handle.*/
+static std::unordered_map<FILE *, HANDLE> popen_map;
+/* Mutex to protect the map.*/
+static std::mutex popen_mtx;
+
+/**
+Creates a FILE* from HANDLE.
+*/
+static FILE *make_fp(HANDLE *handle, const char *mode)
+{
+  int flags = 0;
+
+  if (mode[0] == REDIRECT_STDOUT)
+    flags |= O_RDONLY;
+  switch (mode[1])
+  {
+  case 't':
+    flags |= _O_TEXT;
+    break;
+  case 'b':
+    flags |= _O_BINARY;
+    break;
+  }
+
+  int fd= _open_osfhandle((intptr_t) *handle, flags);
+  if (fd < 0)
+    return NULL;
+  FILE *fp= fdopen(fd, mode);
+  if (!fp)
+  {
+    /* Closing file descriptor also closes underlying handle.*/
+    close(fd);
+    *handle= 0;
+  }
+  return fp;
+}
+
+/** A home-backed version of popen(). */
+extern "C" FILE *my_win_popen(const char *cmd, const char *mode)
+{
+  FILE *fp(0);
+  char type= mode[0];
+  HANDLE parent_pipe_end(0);
+  HANDLE child_pipe_end(0);
+  PROCESS_INFORMATION pi{};
+  STARTUPINFO si{};
+  std::string command_line;
+
+  /* Create a pipe between this and child process.*/
+  SECURITY_ATTRIBUTES sa_attr{};
+  sa_attr.nLength= sizeof(SECURITY_ATTRIBUTES);
+  sa_attr.bInheritHandle= TRUE;
+  switch (type)
+  {
+  case REDIRECT_STDIN:
+    if (!CreatePipe(&child_pipe_end, &parent_pipe_end, &sa_attr, 0))
+      goto error;
+    break;
+  case REDIRECT_STDOUT:
+    if (!CreatePipe(&parent_pipe_end, &child_pipe_end, &sa_attr, 0))
+      goto error;
+    break;
+  default:
+    /* Unknown mode, éxpected "r", "rt", "w", "wt" */
+    abort();
+  }
+  if (!SetHandleInformation(parent_pipe_end, HANDLE_FLAG_INHERIT, 0))
+    goto error;
+
+  /* Start child process with redirected output.*/
+
+  si.cb= sizeof(STARTUPINFO);
+  si.hStdError= GetStdHandle(STD_ERROR_HANDLE);
+  si.hStdOutput= (type == REDIRECT_STDOUT) ? child_pipe_end
+                                           : GetStdHandle(STD_OUTPUT_HANDLE);
+  si.hStdInput= (type == REDIRECT_STDIN) ? child_pipe_end
+                                         : GetStdHandle(STD_INPUT_HANDLE);
+
+  si.dwFlags|= STARTF_USESTDHANDLES;
+  command_line.append("cmd.exe /c ").append(cmd);
+
+  if (!CreateProcess(0, (LPSTR) command_line.c_str(), 0, 0, TRUE, 0, 0, 0, &si,
+                     &pi))
+    goto error;
+
+  CloseHandle(pi.hThread);
+  CloseHandle(child_pipe_end);
+  child_pipe_end= 0;
+
+  fp= make_fp(&parent_pipe_end, mode);
+  if (fp)
+  {
+    std::unique_lock<std::mutex> lk(popen_mtx);
+    popen_map[fp]= pi.hProcess;
+    return fp;
+  }
+
+error:
+  for (auto handle : { parent_pipe_end, child_pipe_end })
+  {
+    if (handle)
+      CloseHandle(handle);
+  }
+
+  if (pi.hProcess)
+  {
+    TerminateProcess(pi.hProcess, 1);
+    CloseHandle(pi.hProcess);
+  }
+  return NULL;
+}
+
+/** A home-backed version of pclose(). */
+
+extern "C" int my_win_pclose(FILE *fp)
+{
+  /* Find process entry for given file pointer.*/
+  std::unique_lock<std::mutex> lk(popen_mtx);
+  HANDLE proc= popen_map[fp];
+  if (!proc)
+  {
+    errno= EINVAL;
+    return -1;
+  }
+  popen_map.erase(fp);
+  lk.unlock();
+
+  fclose(fp);
+
+  /* Wait for process to complete, return its exit code.*/
+  DWORD ret;
+  if (WaitForSingleObject(proc, INFINITE) || !GetExitCodeProcess(proc, &ret))
+  {
+    ret= -1;
+    errno= EINVAL;
+  }
+  CloseHandle(proc);
+  return ret;
+}