Based on recent work and discussions here, I've checked in changes that implement tss cleanup in statically linked Boost.Thread on Win32 (for VC++ only, so far). I've also used the opportunity to rewrite the rest of the Win32 tss cleanup code and update it to use the Boost copyright. In the process, I eliminated threadmon.cpp, threadmon.hpp, and pe_tls.cpp. They've been replaced by tss_hooks.cpp, tss_hooks.hpp, tss_pe.cpp, and tss_dll.cpp. As suggested by Roland in his posted sample code, the default for VC++ linking of Boost.Threads is now static linking. I also changed all non-Win32 platforms to use static linking by default. I realize this is very late in the regression testing process to make these changes, but it seemed too good an opportunity to miss. If you disagree, please let me know. If the changes cause problems or it turns out to be a wrong decision in people's opinions here, I'll back the changes out quickly until after the release. In any case, many, many thanks to Aaron and Roland for working out this solution and making static linking possible on Win32. Mike
On Thu, 5 Aug 2004 14:35:02 -0400 Michael Glassford <glassfordm@hotmail.com> wrote:
I've also used the opportunity to rewrite the rest of the Win32 tss cleanup code and update it to use the Boost copyright. In the process, I eliminated threadmon.cpp, threadmon.hpp, and pe_tls.cpp. They've been replaced by tss_hooks.cpp, tss_hooks.hpp, tss_pe.cpp, and tss_dll.cpp.
I just tried to run it on my machine. I observed the following: 1) Some CRLF issues. Some of the files (tss_pe.cpp) have strange CRLF settings. There are single CR's and other stange CRLF sequences. I think this was because of editing with MSVC. I can send you a cleaned version. However what do you need? Unix (LF only) or DOS/WINDOWS (CRLF)? 2) The config.hpp got wrong: The lines # #if defined(_DLL) # #else # #endif should read: # if defined(_DLL) # else # endif else a preprocessor error is emited. 3) I noticed that the call to //: on_process_enter(); has been commented out. I remeber you were saying it is not necessary. This might very well be true. But in order to tackle the leak I even would suggest leaving it in and also propose the following (extended) scheme for discussion. on_process_init(); getting called before the first constructor on platforms where possible (and I strong believe there is a way on most) This could be used when available to do the global allocation of the tss_data struct. on_process_enter(); getting called after global ctors but just before entering main or immediately on begin of main on_process_exit(); getting called just before global dtors are running on_process_term(); getting called after global dtors on platforms where possible. Ideally this should run when all threads have ended. This could also be the place to safely deallocate what is now causing a memory leak. The on_process_init and on_process_term should be regarded optional but improve memory consistency where possible. On platforms where this is not possible it should revert to the current behaviour. (Allocation on first usage.) on_process_enter and on_process_exit should be mandatory. Altough in the current implementation on_process_enter seems not to be really necessary, but I think it would be a benefit to have for other library writers which will need to make sure to have an entry point that is guaranted to run after the last ctor. (They could register their handlers in a call to a global ctor e.g. ) Also they are easy to emulate on any system: void main() { on_process_enter(); ... on_process_term(); } What di you think? Roland
Roland wrote:
On Thu, 5 Aug 2004 14:35:02 -0400 Michael Glassford <glassfordm@hotmail.com> wrote:
I've also used the opportunity to rewrite the rest of the Win32 tss cleanup code and update it to use the Boost copyright. In the process, I eliminated threadmon.cpp, threadmon.hpp, and pe_tls.cpp. They've been replaced by tss_hooks.cpp, tss_hooks.hpp, tss_pe.cpp, and tss_dll.cpp.
I just tried to run it on my machine. I observed the following:
1) Some CRLF issues.
Darn, not that again.
Some of the files (tss_pe.cpp) have strange CRLF settings. There are single CR's and other stange CRLF sequences. I think this was because of editing with MSVC.
I think it must be the msvc 6 editor. I don't usually use it anymore, but did once or twice. For the most part I use msvc 7 editor, which seems to be ok (?) unless the file has inconsistent line endings to start with.
I can send you a cleaned version. However what do you need? Unix (LF only) or DOS/WINDOWS (CRLF)?
Thanks, but I've fixed the problems that I found.
2) The config.hpp got wrong: The lines # #if defined(_DLL) # #else # #endif
should read: # if defined(_DLL) # else # endif
else a preprocessor error is emited.
Fixed. Odd that I didn't see any error.
3) I noticed that the call to //: on_process_enter(); has been commented out.
I commented this out while experimenting, and didn't notice it until after I had checked in the file. I didn't check in a fix right away because on_process_enter() doesn't currently do anything. It's fixed now.
I remeber you were saying it is not necessary. This might very well be true. But in order to tackle the leak I even would suggest leaving it in and also propose the following (extended) scheme for discussion.
on_process_init(); getting called before the first constructor on platforms where possible (and I strong believe there is a way on most) This could be used when available to do the global allocation of the tss_data struct.
on_process_enter(); getting called after global ctors but just before entering main or immediately on begin of main
on_process_exit(); getting called just before global dtors are running
on_process_term(); getting called after global dtors on platforms where possible. Ideally this should run when all threads have ended. This could also be the place to safely deallocate what is now causing a memory leak.
The on_process_init and on_process_term should be regarded optional but improve memory consistency where possible. On platforms where this is not possible it should revert to the current behaviour. (Allocation on first usage.)
on_process_enter and on_process_exit should be mandatory. Altough in the current implementation on_process_enter seems not to be really necessary, but I think it would be a benefit to have for other library writers which will need to make sure to have an entry point that is guaranted to run after the last ctor. (They could register their handlers in a call to a global ctor e.g. ) Also they are easy to emulate on any system: void main() { on_process_enter(); .... on_process_term(); }
Sounds OK. Mike
Michael Glassford wrote:
Based on recent work and discussions here, I've checked in changes that implement tss cleanup in statically linked Boost.Thread on Win32 (for VC++ only, so far).
[...]
Thanks a lot for your work (the same to all the others who were involved in the never-ending discussions over the last days and weeks which I personally wasn't able to participate in any more for time reasons :) I strongly believe that static linking is very crucial for many being able to use boost.thread at all! Stefan
Michael Glassford wrote:
As suggested by Roland in his posted sample code, the default for VC++ linking of Boost.Threads is now static linking. I also changed all non-Win32 platforms to use static linking by default.
I still get an error from auto_link when including something from boost.thread and using any non-dynamic runtime: C:\views_cvs\boost\boost/config/auto_link.hpp(269): catastrophic error: #error directive: "Mixing a dll boost library with a static runtime is a really bad idea..." # error "Mixing a dll boost library with a static runtime is a really bad idea..." Is this intended? Stefan
Stefan Slapeta wrote:
Michael Glassford wrote:
As suggested by Roland in his posted sample code, the default for VC++ linking of Boost.Threads is now static linking. I also changed all non-Win32 platforms to use static linking by default.
I still get an error from auto_link when including something from boost.thread and using any non-dynamic runtime:
C:\views_cvs\boost\boost/config/auto_link.hpp(269): catastrophic error: #error directive: "Mixing a dll boost library with a static runtime is a really bad idea..." # error "Mixing a dll boost library with a static runtime is a really bad idea..."
Is this intended?
I don't see how this can happen unless: a) your boost/thread/detail/config.hpp is out of date, or b) you're defining BOOST_THREAD_USE_DLL or BOOST_DYN_LINK. If you're sure neither of these is the case, could you determine which branch is being chosen in the complicated #if near the top of boost/thread/detail.config.hpp that looks like the following? #if defined(BOOST_HAS_WINTHREADS) # if defined(BOOST_THREAD_BUILD_DLL) //Build dll # define BOOST_THREAD_DECL __declspec(dllexport) # elif defined(BOOST_THREAD_BUILD_LIB) //Build lib # define BOOST_THREAD_DECL # elif defined(BOOST_THREAD_USE_DLL) //Use dll # define BOOST_THREAD_DECL __declspec(dllimport) # define BOOST_DYN_LINK # elif defined(BOOST_THREAD_USE_LIB) //Use lib # define BOOST_THREAD_DECL # else //Use default # if defined(BOOST_MSVC) //For VC++, choose according to threading library setting # if defined(_DLL) //Threading library is dll: use Boost.Threads dll # define BOOST_THREAD_USE_DLL # define BOOST_THREAD_DECL __declspec(dllimport) # define BOOST_DYN_LINK # else //Threading library is lib: used Boost.Threads lib # define BOOST_THREAD_USE_LIB # define BOOST_THREAD_DECL # endif # else //For compilers not yet supporting auto-tss cleanup //with Boost.Threads lib, use Boost.Threads dll # define BOOST_THREAD_USE_DLL # define BOOST_THREAD_DECL __declspec(dllimport) # define BOOST_DYN_LINK # endif # endif #else # define BOOST_THREAD_DECL # if defined(BOOST_THREAD_USE_LIB) //Use dll # define BOOST_THREAD_USE_DLL # define BOOST_DYN_LINK # elif defined(BOOST_THREAD_USE_DLL) //Use lib # define BOOST_THREAD_USE_LIB # else //Use default # define BOOST_THREAD_USE_LIB # endif #endif // BOOST_HAS_WINTHREADS Thanks, Mike
On Tue, 10 Aug 2004 23:01:08 +0200 Stefan Slapeta <stefan_nospam_@slapeta.com> wrote:
Michael Glassford wrote:
I don't see how this can happen unless: [...]
It works for debug as you say, but not for release! Just try to compile on VC 7.1 with the "Multithreaded" runtime. However, I've not found out yet where exactly the problem is located...
I tried this, but ran into another problem: At the moment the CVS seems to be a little split brain, because altough it compiles the library to 1.31 the headers attempt to link in 1.32. I think I've seen a post that this is already solved, but unfortunately not yet for the anonymous access :-( So I can't try it in the moment. Roland
participants (5)
-
Bronek Kozicki -
Michael Glassford -
Roland -
Stefan Slapeta -
Stefan Slapeta