GNOME Bugzilla – Bug 693646
Fix Windows gspawn helper programs with newer Microsoft CRTs
Last modified: 2013-07-31 10:47:52 UTC
Hi, I was building and using glib-compile-resources on Windows using Visual C++ 2008 (and later), since there are more and more GNOME libraries/applications that are now written to use the gresource framework that Alex introduced at around 2.30 or 2.32 (or so), and found that it crashes in running gspawn-win32-helper-console.exe in my builds but interestingly not in the OpenSUSE Build Service (OBS, i.e. MinGW) builds. The same also holds for 64-bit builds too. Upon some further investigation, the gspawn-win32-helper-console.exe is called as it is used to call (spawn) xmllint.exe to process the XML in the process. Also, the newer Microsoft CRTs (since circa 8.0/2005) have much stricter (even paranoid) checks on the usage of close(), _get_osfhandle() etc, such as doubly calling close() on a single file descriptor and using an invalid file descriptor. This is what ultimately causes gspawn-win32-helper-console.exe (and the other win32 spawn helper programs) to abort. As a result, I have attempted to come up with a patch so that we can be more careful about the use of the file descriptors in gspawn-win32-helper.c, so that this kind of situation can be avoided (on newer Microsoft CRTs), and that programs using gspawn built with newer Microsoft CRTs can run normally. This will, thus, fix glib-compile-resources that is linked to newer Microsoft CRTs-I have checked it by generating GTK+'s demos/widget-factory/widget_factory_resources.c and gtksourceview's gtksourceview/gtksourceview-resources.c. With blessings, Thank you!
Created attachment 235775 [details] [review] Patch to fix gspawn-win32-helper.c with newer Microsoft CRTs
Created attachment 235779 [details] [review] Patch to fix gspawn-win32-helper.c with newer Microsoft CRTs (take II) Hi, Sorry, the previous patch missed the last item to close, causing a file descriptor leak. This fixes that problem as well. This also adds more comments to the changes to tell people what goes on with these changes. This patch would be needed on both the master and glib-2-34 branch, and would also allow the spawn-singlethread and spawn-multithreaded tests to pass (except obviously the /gthread/spawn-script test, which IMHO is unixy) after changing "test-spawn-echo" to "test-spawn-echo.exe". With blessings, thank you!
Shouldn't the #include <crtdbg.h> also be guarded in an #if _MSC_VER >= 1400 section? The win32 api functions _set_invalid_parameter_handler and _CrtSetReportMode are only called in the MSVC specific code, so the include shouldn't be needed when using MinGW
Created attachment 237406 [details] [review] Patch to fix gspawn-win32-helper.c with newer Microsoft CRTs (take III) Hi, This incorporates Erik's comments on the patch, and makes the #ifdef's a little bit more consistent. With blessings, thank you!
Review of attachment 237406 [details] [review]: Hi, I have pushed this patch in the master branch and the glib-2-34 branch. Please feel free to reopen the bug if this should cause any problems. With blessings, thank you!
*** Bug 552733 has been marked as a duplicate of this bug. ***