Boost C++ Libraries: Ticket #8817: Boost Thread Windows CE _createthreadex handling breaks mingw w64

attachment set

Sat, 13 Jul 2013 10:38:10 GMT

attachment → thread.cpp.patch

patch for libs\thread\src\win32\thread.cpp

attachment set

Sat, 13 Jul 2013 18:51:42 GMT

attachment → wince.patch

patch which fixes boost threads and also moves wince configs to win32.hpp

Sat, 13 Jul 2013 18:53:34 GMT

After digging through the Boost configs, it seems that these defines are part of visualc.hpp:

#if defined(_WIN32_WCE) || defined(UNDER_CE)
// Windows CE does not have a conforming signature for swprintf
#  define BOOST_NO_SWPRINTF
#endif
// we have ThreadEx or GetSystemTimeAsFileTime unless we're running WindowsCE
#if !defined(_WIN32_WCE) && !defined(UNDER_CE)
#  define BOOST_HAS_THREADEX
#  define BOOST_HAS_GETSYSTEMTIMEASFILETIME
#endif

This probably should be included in win32.hpp, or perhaps in a new platform config file wince.hpp.

Perhaps this is a better option? I still think the changed no _beginthreadex may work betterh, though, as we probably shouldn't be typdef'ing uintptr_t anyways.

Added an updated patch which fixes both win32/thread.hpp as well as the threadex wince platform config.

owner, component changed

viboes — Sat, 03 Aug 2013 05:25:54 GMT

owner changed from Anthony Williams to John Maddock
component thread → config

I think this should be fixed in Boost.Config.

Sat, 03 Aug 2013 07:28:35 GMT

The problem is in Boost.Config and Boost.Thread.

Obviously the work-arounds for Windows CE should not be used when compiling with MinGW (unless targeting Windows CE specifically), so Boost.Config must be changed.

IMO Boost.Thread should also be changed because defining uintptr_t really shouldn't be done at all. I would imagine any modern compiler targeting Windows CE might have uintptr_t defined (it is standard C++ after all), in which case the compile will fail again.

cc set

Wed, 14 Aug 2013 12:26:12 GMT

cc jim@… added

attachment set

Thu, 15 Aug 2013 11:21:34 GMT

attachment → minimal-thread.cpp.patch

Minimal patch to libs/thread/src/win32/thread.cpp: include <boost/cstdint.hpp> and yank the uintptr_t typedef.

owner, component changed; keywords set

Thu, 15 Aug 2013 11:32:35 GMT

keywords MinGW uintptr_t added
owner changed from John Maddock to Anthony Williams
component config → thread

Added a minimal patch to libs/thread/src/win32/thread.cpp which I verified to work with MinGW gcc 4.8, and both boost trunk and release SVN branches. Using <boost/cstdint.hpp> for the typedef for uintptr_t maximizes confidence that it will work for WinCE as well. (And should WinCE fail, pushes the fix from thread to config where it belongs.) With this change being only to thread.cpp, I'm changing the component from config back to thread.

attachment set

Thu, 15 Aug 2013 17:21:54 GMT

attachment → mingw.config.patch

patch only for config detection with MinGW and WinCE

Thu, 15 Aug 2013 17:28:20 GMT

Should the code for detecting BOOST_NO_SWPRINTF, BOOST_HAS_THREADEX, and BOOST_HAS_GETSYSTEMTIMEASFILETIME be moved to win32.hpp (from visualc.hpp)? These are platform-specific defines, not compiler-specific.

You should be able to verify this with Mingw testing this code:

#include <boost/config>
#include <iostream>
int main(void)
{
#ifndef BOOST_HAS_THREADEX
    // This should be true for any compiler targeting Windows and can use the WinAPI, which MinGW can
    std::cout << "didn't detect BOOST_HAS_THREADEX" << std::endl;
#else
    std::cout << "has BOOST_HAS_THREADEX" << std::endl;
#endif
}

As-is, MinGW will still not detect these correctly because it doesn't (and shouldn't) include visualc.hpp. However, when targeting Windows it should and will include win32.hpp and detection will work as intended.

I've added a smaller patch which only applies these fixes to Config, so applying minimal-thread.cpp.patch and mingw.config.patch should fully fix Boost.Config and Boost.Thread.

Thu, 15 Aug 2013 17:37:03 GMT

Andrew: I have to say I didn't look too closely at your proposed solution, but was working backwards from (a lot of) failed regression tests and found this ticket. I'd say (1) you might want to split this into a separate ticket for config, and (2) present it as a specific failed regression test, or a missing regression test.

Tue, 27 Aug 2013 22:45:20 GMT

I'm pretty sure was caused by [82777] on thread.cpp, which changed the <thread.hpp> include to <thread_only.hpp>. Looking for a way to change this ticket's owner, but apparently don't have permission. Will look for him on lists (viboes) and contact directly.

viboes — Sat, 31 Aug 2013 10:16:24 GMT

Replying to Jim Bell <jim@…>:

Added a minimal patch to libs/thread/src/win32/thread.cpp which I verified to work with MinGW gcc 4.8, and both boost trunk and release SVN branches. Using <boost/cstdint.hpp> for the typedef for uintptr_t maximizes confidence that it will work for WinCE as well. (And should WinCE fail, pushes the fix from thread to config where it belongs.) With this change being only to thread.cpp, I'm changing the component from config back to thread.

This code was working well on mingw 32. What has been changed in mingw 64? Have you tested your patch on mingw 32?

owner, status, version, milestone changed

viboes — Sun, 01 Sep 2013 07:26:48 GMT

owner changed from Anthony Williams to viboes
status new → assigned
version Boost Development Trunk → Boost 1.54.0
milestone To Be Determined → Boost 1.55.0

Committed revision [85540].

Sun, 01 Sep 2013 21:43:46 GMT

I'm reverting the trunk patch on my MinGW-gcc testers and will begin to re-cycle those tests.

Mon, 09 Sep 2013 10:22:15 GMT

It appears that all trunk tests have run with revisions above [85540], and without issue. I think this can go to the release branch and close the ticket.

Mon, 09 Sep 2013 16:21:41 GMT

I submitted the other portion of this issue with Boost.Config as #9095. This should allow proper detection of Windows platform features on compilers other than VS by Boost.Config.

status changed; resolution set

viboes — Mon, 09 Sep 2013 17:02:08 GMT

status assigned → closed
resolution → fixed

Committed revision [85621].