Fix gspawn-win32-helper.c with newer Microsoft CRTs

The newer Microsoft CRTs (8.0/2005 and later) impose much stricter
(paranoid) checks on close() being doubly called and the use of
invalid file descriptors.  This makes the calls on the file descriptors
use more caution when using them and only call close() when necessary.

This also adds an (empty) invalid parameter handler* as required by the
newer Microsoft CRTs to prevent the system from aborting the process
when we are checking whether a file descriptor is valid.

[*]: http://msdn.microsoft.com/en-us/library/a9yf33zb.aspx

https://bugzilla.gnome.org/show_bug.cgi?id=693646

diff --git a/glib/gspawn-win32-helper.c b/glib/gspawn-win32-helper.c
index 40108bb..80e6757 100644 (file)
--- a/glib/gspawn-win32-helper.c
+++ b/glib/gspawn-win32-helper.c
@@ -23,6 +23,15 @@
 
 #include <fcntl.h>
 
+/* For _CrtSetReportMode, we don't want Windows CRT (2005 and later)
+ * to terminate the process if a bad file descriptor is passed into
+ * _get_osfhandle.  The newer MS CRT's are picky
+ * on double close()'s and bad file descriptors.
+ */
+#if (defined (_MSC_VER) && _MSC_VER >= 1400)
+#include <crtdbg.h>
+#endif
+
 #undef G_LOG_DOMAIN
 #include "glib.h"
 #define GSPAWN_HELPER
@@ -147,6 +156,32 @@ protect_wargv (wchar_t  **wargv,
   return argc;
 }
 
+#if (defined (_MSC_VER) && _MSC_VER >= 1400)
+/*
+ * This is the (empty) invalid parameter handler
+ * that is used for Visual C++ 2005 (and later) builds
+ * so that we can use this instead of the system automatically
+ * aborting the process, as the newer MS CRTs are more picky
+ * about double close()'s and bad/invalid file descriptors.
+ *
+ * This is necessary as we use _get_oshandle() to check the validity
+ * of the file descriptors as we close them, so when an invalid file
+ * descriptor is passed into that function as we check on it, we get
+ * -1 as the result, instead of the gspawn helper program aborting.
+ */
+void myInvalidParameterHandler(
+   const wchar_t * expression,
+   const wchar_t * function,
+   const wchar_t * file,
+   unsigned int line,
+   uintptr_t pReserved
+)
+{
+  return;
+}
+#endif
+
+
 #ifndef HELPER_CONSOLE
 int _stdcall
 WinMain (struct HINSTANCE__ *hInstance,
@@ -173,6 +208,21 @@ main (int ignored_argc, char **ignored_argv)
   _startupinfo si = { 0 };
   char c;
 
+  /* store up the file descriptors to close */
+  GSList *fd_toclose = NULL;
+  GSList *last_item = NULL;
+
+#if (defined (_MSC_VER) && _MSC_VER >= 1400)
+  /* set up our empty invalid parameter handler */
+  _invalid_parameter_handler oldHandler, newHandler;
+  newHandler = myInvalidParameterHandler;
+  oldHandler = _set_invalid_parameter_handler(newHandler);
+
+  /* Disable the message box for assertions. */
+  _CrtSetReportMode(_CRT_ASSERT, 0);
+#endif
+
+
   g_assert (__argc >= ARG_COUNT);
 
   /* Fetch the wide-char argument vector */
@@ -217,7 +267,7 @@ main (int ignored_argc, char **ignored_argv)
       if (fd != 0)
        {
          dup2 (fd, 0);
-         close (fd);
+         fd_toclose = g_slist_append (fd_toclose, GINT_TO_POINTER (fd));
        }
     }
   else
@@ -226,7 +276,7 @@ main (int ignored_argc, char **ignored_argv)
       if (fd != 0)
        {
          dup2 (fd, 0);
-         close (fd);
+         fd_toclose = g_slist_append (fd_toclose, GINT_TO_POINTER (fd));
        }
     }
 
@@ -238,7 +288,7 @@ main (int ignored_argc, char **ignored_argv)
       if (fd != 1)
        {
          dup2 (fd, 1);
-         close (fd);
+         fd_toclose = g_slist_append (fd_toclose, GINT_TO_POINTER (fd));
        }
     }
   else
@@ -247,7 +297,7 @@ main (int ignored_argc, char **ignored_argv)
       if (fd != 1)
        {
          dup2 (fd, 1);
-         close (fd);
+         fd_toclose = g_slist_append (fd_toclose, GINT_TO_POINTER (fd));
        }
     }
 
@@ -259,7 +309,7 @@ main (int ignored_argc, char **ignored_argv)
       if (fd != 2)
        {
          dup2 (fd, 2);
-         close (fd);
+         fd_toclose = g_slist_append (fd_toclose, GINT_TO_POINTER (fd));
        }
     }
   else
@@ -268,7 +318,7 @@ main (int ignored_argc, char **ignored_argv)
       if (fd != 2)
        {
          dup2 (fd, 2);
-         close (fd);
+         fd_toclose = g_slist_append (fd_toclose, GINT_TO_POINTER (fd));
        }
     }
 
@@ -287,7 +337,22 @@ main (int ignored_argc, char **ignored_argv)
   if (__argv[ARG_CLOSE_DESCRIPTORS][0] == 'y')
     for (i = 3; i < 1000; i++) /* FIXME real limit? */
       if (i != child_err_report_fd && i != helper_sync_fd)
-       close (i);
+        if (_get_osfhandle (i) != -1)
+          fd_toclose = g_slist_append (fd_toclose, GINT_TO_POINTER (i));
+
+  /* ...so we won't get the nasty off-by-1 file descriptor leak */
+  g_slist_append (fd_toclose, NULL);
+  last_item = g_slist_last (fd_toclose);
+
+  /* now close all the file descriptors as necessary */
+  if (fd_toclose != NULL && last_item != NULL)
+  {
+    for ( ; fd_toclose != last_item; fd_toclose = fd_toclose->next)
+      close (GPOINTER_TO_INT(fd_toclose->data));
+  }
+  g_slist_free (last_item);
+  g_slist_free (fd_toclose);
+
 
   /* We don't want our child to inherit the error report and
    * helper sync fds.