[thread] tss_cleanup_implemented issue
In the most recent thread library changes from a day or so ago, the set_tss_data function was added to win32/thread.cpp. This function calls tss_cleanup_implemented to force some support for TSS cleanup to be present. Unfortunately, since set_tss_data is in the .cpp file (rather than being an inline function in a .hpp file) this requires tss_cleanup_implemented to always be present for WinThread versions of the static library, regardless of whether or not TSS is actually used. Since WinCE does not currently have a nifty automated way to clean up TSS in static builds, this means that there is always a link error when the static library is used (even when no TSS data is allocated). As an alternative, could the call to tss_cleanup_implemented() be moved to the thread_specific_ptr::reset() and thread_specific_ptr::release() functions in win32/tss.hpp, so that the link error only shows up if some TSS data is created? Incidentally, while looking through this, I had a question about src/tss_null.cpp. Does this file actually accomplish anything at the moment? It appears that it is intended to patch in an empty version of tss_cleanup_implemented() for WinThread builds when _MSC_VER is not defined. However, the #if block around the function definition includes the clause (defined(BOOST_THREAD_BUILD_LIB) || defined(BOOST_THREAD_TEST)). As far as I can tell, this file is not included in any static library build by thread/build/Jamfile.v2 and none of the test files declare BOOST_THREAD_TEST. So I'm not sure when this function definition would actually come into play. Just curious. Thanks, -Dave
David Deakins wrote:
In the most recent thread library changes from a day or so ago, the set_tss_data function was added to win32/thread.cpp. This function calls tss_cleanup_implemented to force some support for TSS cleanup to be present.
This is by intent.
Unfortunately, since set_tss_data is in the .cpp file (rather than being an inline function in a .hpp file) this requires tss_cleanup_implemented to always be present for WinThread versions of the static library, regardless of whether or not TSS is actually used.
I agree, this is somewhat unfortunate. But the intent is that the application programmer always can define this function in her code, if she knows that automatic tss_cleanup either is not needed, or taken care of by th application itself. (By calling into at_thread_exit,....)
Since WinCE does not currently have a nifty automated way to clean up TSS in static builds, this means that there is always a link error when the static library is used (even when no TSS data is allocated). As an alternative, could the call to tss_cleanup_implemented() be moved to the thread_specific_ptr::reset() and thread_specific_ptr::release() functions in win32/tss.hpp, so that the link error only shows up if some TSS data is created?
I am afraid this is not possible since using thread implies linking to tls.
Incidentally, while looking through this, I had a question about src/tss_null.cpp. Does this file actually accomplish anything at the moment? It appears that it is intended to patch in an empty version of tss_cleanup_implemented() for WinThread builds when _MSC_VER is not defined. However, the #if block around the function definition includes the clause (defined(BOOST_THREAD_BUILD_LIB) || defined(BOOST_THREAD_TEST)). As far as I can tell, this file is not included in any static library build by thread/build/Jamfile.v2 and none of the test files declare BOOST_THREAD_TEST. So I'm not sure when this function definition would actually come into play. Just curious.
This file is currently used in the test suite. It allows for static linking even if automatic cleanup is not available. BUT: the tests are taking care of this, i.e they test for cleanup working or not. For static linking you basically can take three approaches: 1) compiler/platform supports automatic cleanup: nothing to do. 2) ... does not support auto cleanup _and_ 2a) you are sure you do not need tss: just define tss_cleanup_implemented (@anthony: is it possible for a thread application not to use tss?) 2b) you need tss: define tss_cleanup_implemented, make a call to on_process_enter() just on begin of main and a call to on_process_exit just before end of main. If you are launching threads _not_ by using the boost api: make a call to on_thread_exit() when the thread ends. If you are using the boost api to launch thread the last step can be omitted. An additional note: The above should be better documented by me of course. Also the above is not totally semantically equivalent with the automatic cleanup in terms of when the respective functions are really called. This needs some rethought. But you should be able to call into the _on_exit function multiple times safely. So at minimum one call sahll be present. Btw.: Perhaps somebody noticed: I have added support for automatic cleanup recently for the mingw compiler. Anyone knowing enough about BCB so we can have this for Borland too? Roland aka speedsnail
Roland Schwarz wrote:
David Deakins wrote:
Incidentally, while looking through this, I had a question about src/tss_null.cpp.
none of the test files declare BOOST_THREAD_TEST. So I'm not sure when this function definition would actually come into play.
This file is currently used in the test suite. It allows for static linking even if automatic cleanup is not available. BUT: the tests are taking care of this, i.e they test for cleanup working or not.
Should the thread/test/Jamfile.v2 declare BOOST_THREAD_TEST then? That would allow the thread regression tests to link correctly for the WinCE regression tests. Then the results of the test would reflect whether cleanup is happening correctly, right? Incidentally, while I was studying the implementation of the cleanup code, I think I ran across a bug in win32/thread.cpp. detail::add_thread_exit_function uses get_current_thread_data() but does not verify whether or not the returned pointer is NULL. If the thread was not created with the boost api, get_current_thread_data() will indeed return a NULL (if I understand correctly) and make_external_thread_data() needs to be called. This is how set_tss_data handles things. Am a correct that add_thread_exit_function is missing this externally-generated thread code? Thanks, -Dave
David Deakins <ddeakins <at> veeco.com> writes:
Roland Schwarz wrote:
David Deakins wrote:
Incidentally, while looking through this, I had a question about src/tss_null.cpp.
This file is currently used in the test suite. It allows for static linking even if automatic cleanup is not available. BUT: the tests are taking care of this, i.e they test for cleanup working or not.
Should the thread/test/Jamfile.v2 declare BOOST_THREAD_TEST then? That would allow the thread regression tests to link correctly for the WinCE regression tests. Then the results of the test would reflect whether cleanup is happening correctly, right?
I thought the test Jamfile was set up to catch this, hence the regression tests for Borland are showing lack of TSS cleanup for the static library, rather than failing to link. I'll investigate.
I think I ran across a bug in win32/thread.cpp. detail::add_thread_exit_function uses get_current_thread_data() but does not verify whether or not the returned pointer is NULL. If the thread was not created with the boost api, get_current_thread_data() will indeed return a NULL (if I understand correctly) and make_external_thread_data() needs to be called.
Yes, you're right. I'll make the change. Anthony
Anthony Williams wrote:
I thought the test Jamfile was set up to catch this, hence the regression tests for Borland are showing lack of TSS cleanup for the static library, rather than failing to link. I'll investigate.
No need to: William Kempf set it up this way: The test is comiled with to tss_null.cpp, which defines tss_cleanup_implemeted, so there is will be a runtime test failure instead of a linking failure. Roland aka speedsnail
Roland Schwarz wrote:
Anthony Williams wrote:
I thought the test Jamfile was set up to catch this, hence the regression tests for Borland are showing lack of TSS cleanup for the static library, rather than failing to link. I'll investigate.
No need to: William Kempf set it up this way: The test is comiled with to tss_null.cpp, which defines tss_cleanup_implemeted, so there is will be a runtime test failure instead of a linking failure.
Except that the V1 jamfile had a <define>BOOST_THREAD_TEST=1 line in the requirements that does not appear to have been ported to the V2 jamfile. So tss_null.cpp will not actually contain any code (without BOOST_THREAD_TEST the tss_cleanup_implemented definition will get #ifdef'd away). I think all that needs to happen is to put a <define>BOOST_THREAD_TEST=1 in the V2 jamfile requirements. Should I make this change? -Dave
David Deakins wrote:
Except that the V1 jamfile had a <define>BOOST_THREAD_TEST=1 line in the requirements that does not appear to have been ported to the V2 jamfile. So tss_null.cpp will not actually contain any code (without BOOST_THREAD_TEST the tss_cleanup_implemented definition will get #ifdef'd away).
I think all that needs to happen is to put a <define>BOOST_THREAD_TEST=1 in the V2 jamfile requirements. Should I make this change?
Hmm, this seems to be true. I was under the impression BOOST_THREAD_BUILD_LIB is defined because the lib will tear this in. But I am not sure anymore. I need to investigate, pls. stand by. Btw.: Since you seem to run CE tests: Do you post your results to the regression table too? Roland aka speedsnail
Roland Schwarz wrote:
Btw.: Since you seem to run CE tests: Do you post your results to the regression table too?
Yes, I help manage the VeecoFTC test installation on the regression test page. Results are posted for msvc8/stlport5.1 on WinXP and on Windows Mobile 5 (WinCE 5). -Dave
David Deakins wrote:
in the V2 jamfile requirements. Should I make this change?
Please don't yet. 1) Your observation was correct. Thank you for having pointed this out. 2) Applying the change causes the borland tests to fail badly, i.e. the tests will run for a _very_ long time until they time out. So please wait until I have looked into the issue. Roland aka speedsnail
Roland Schwarz wrote:
David Deakins wrote:
in the V2 jamfile requirements. Should I make this change?
Please don't yet.
2) Applying the change causes the borland tests to fail badly, i.e. the tests will run for a _very_ long time until they time out.
So please wait until I have looked into the issue.
Hi Roland, Just wanted to see if you had made any progress on understand the Borland issues. Thanks, -Dave
David Deakins wrote:
Just wanted to see if you had made any progress on understand the Borland issues.
Not much so far, unfortunately. :-( I would be glad for any pointers to information that could be interesting for this problem: E.g. ilink32 description (beyond what is in the bcb docs) Information, how one can create coff sections in source files. ... Roland aka speedsnail Btw.: I have not yet given up ... ;-)
Roland Schwarz <roland.schwarz <at> chello.at> writes:
Btw.: Perhaps somebody noticed: I have added support for automatic cleanup recently for the mingw compiler. Anyone knowing enough about BCB so we can have this for Borland too?
I did notice, thanks. I've been talking to the Visual Studio team at Microsoft about how to get the cleanup working with the 64-bit Microsoft compilers, so hopefully that will work soon, too. Anthony
Hi all,
In the most recent thread library changes from a day or so ago, the set_tss_data function was added to win32/thread.cpp.
Perhaps unrelated, but since yesterday or so the Wave tests are failing because of a segfault during application termination somewhere in the middle of the tss code (see my other post '[Thread] Wave segfaults at application cleanup in thread library' for details). Could anybody shed some light on this? Regards Hartmut
Hartmut Kaiser wrote:
Hi all,
In the most recent thread library changes from a day or so ago, the set_tss_data function was added to win32/thread.cpp.
Perhaps unrelated, but since yesterday or so the Wave tests are failing because of a segfault during application termination somewhere in the middle of the tss code (see my other post '[Thread] Wave segfaults at application cleanup in thread library' for details).
Could anybody shed some light on this?
Hmm, this is really strange. Since the wave tests were failing for quite some time on windows for mingw compiler, I looked into the problem and fixed it last weekend: I implemented the automatic tss cleanup code for statically linked thread lib. Then I ran the tests, and they were all fine. Also the regression results were fine too. Then somewhere in the middle of the week these strange error appeared. I will try to look into the issue. Roland aka speedsnail
participants (4)
-
Anthony Williams -
David Deakins -
Hartmut Kaiser -
Roland Schwarz