From c7996825ce999a99f2d73722419c0d11d0b52238 Mon Sep 17 00:00:00 2001 From: Chun-wei Fan Date: Tue, 26 Feb 2013 12:38:57 +0800 Subject: [PATCH] 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 --- glib/gspawn-win32-helper.c | 79 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 72 insertions(+), 7 deletions(-) diff --git a/glib/gspawn-win32-helper.c b/glib/gspawn-win32-helper.c index 40108bb..80e6757 100644 --- a/glib/gspawn-win32-helper.c +++ b/glib/gspawn-win32-helper.c @@ -23,6 +23,15 @@ #include +/* 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 +#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. -- 2.7.4