Race condition in boost::spirit?
I'm using vc71 with multi-threaded runtime. Sometimes then boost::spirit code is accessed from different threads simultaneously i have access violation at boost/spirit/core/non_terminal/impl/grammar.ipp (255): # ifdef BOOST_SPIRIT_THREADSAFE static boost::thread_specific_ptr<ptr_t> tld_helper; // <-- Here if (!tld_helper.get()) tld_helper.reset(new ptr_t); ptr_t &helper = *tld_helper; # else According to callstack "tss_data" from threads/src/tss.cpp is dereferenced then it still is not initialized.
stas garifulin wrote:
I'm using vc71 with multi-threaded runtime. Sometimes then boost::spirit code is accessed from different threads simultaneously i have access violation at boost/spirit/core/non_terminal/impl/grammar.ipp (255):
# ifdef BOOST_SPIRIT_THREADSAFE static boost::thread_specific_ptr<ptr_t> tld_helper; // <-- Here if (!tld_helper.get()) tld_helper.reset(new ptr_t); ptr_t &helper = *tld_helper; # else
I am afraid the usage of thread_specific_ptr as a local static object is "implementation dependant". The constructors might be called when "first time control passes through its declaration". This would be bad. As far as I know thread_specific_ptr is expected to be constructed before main. Also what does "first time" mean when MT is involved? The standard does not know about MT. Will it be called twice? I think the poiner should be moved to file scope. Roland
Roland Schwarz wrote:
stas garifulin wrote:
I'm using vc71 with multi-threaded runtime. Sometimes then boost::spirit code is accessed from different threads simultaneously i have access violation at boost/spirit/core/non_terminal/impl/grammar.ipp (255):
# ifdef BOOST_SPIRIT_THREADSAFE static boost::thread_specific_ptr<ptr_t> tld_helper; // <-- Here if (!tld_helper.get()) tld_helper.reset(new ptr_t); ptr_t &helper = *tld_helper; # else
I am afraid the usage of thread_specific_ptr as a local static object is "implementation dependant". The constructors might be called when "first time control passes through its declaration". This would be bad. As far as I know thread_specific_ptr is expected to be constructed before main. Also what does "first time" mean when MT is involved? The standard does not know about MT. Will it be called twice?
I think the poiner should be moved to file scope.
Some additional information: Altough the thread_specific_ptr ctor tries to guard itself against concurrent access of its underlying tss_data struct, it is not prepared to be multiply called. Declaring a local static object on the other hand protects multiple calls to the ctor by setting a local flag and scheduling a call for its dtor to atexit. It is exactly this setting of the flag that poses a race condition, since it is not protected by a critical section. Altough it might work to internally protect the ctor against multiple invocation, this would not solve the race condition. The side effect would be to schedule two calls to atexit for the destructor, which is equally bad. The broader question is: Can local static objects ever be made thread safe? Or should they be avoided at all in MT aware code? Roland
Roland
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Roland Schwarz wrote:
I am afraid the usage of thread_specific_ptr as a local static object is "implementation dependant". The constructors might be called when "first time control passes through its declaration". This would be bad. As far as I know thread_specific_ptr is expected to be constructed before main. Also what does "first time" mean when MT is involved? The standard does not know about MT. Will it be called twice?
I think the poiner should be moved to file scope.
Some additional information: Altough the thread_specific_ptr ctor tries to guard itself against concurrent access of its underlying tss_data struct, it is not prepared to be multiply called.
Declaring a local static object on the other hand protects multiple calls to the ctor by setting a local flag and scheduling a call for its dtor to atexit. It is exactly this setting of the flag that poses a race condition, since it is not protected by a critical section. Altough it might work to internally protect the ctor against multiple invocation, this would not solve the race condition. The side effect would be to schedule two calls to atexit for the destructor, which is equally bad.
Could you provide a patch? Regards Hartmut
Hartmut Kaiser wrote:
Could you provide a patch?
As you know: It is easy to suggest a solution if you know nothing about the problem. And I definitely do not know enough about spirit. To be precise: I have no idea how the *.ipp is getting into play. So please understand that I am only showing a way: Say you have: void foo() { static boost::thread_specific_ptr<mytype> p; p->myval; } I suggest instead to have something along the lines: namespace { // to keep things private as possible boost::thread_specific_ptr<mytype> foo_p; }; void foo() { foo_p->myval; } Since I expect, that it will be not of much difference if you put the tss into anonymous namespace this should force the ctor beeing called once before main. The point is that thread_specific_ptr needs to be a global object. Does this help? Roland
Roland Schwarz wrote:
I suggest instead to have something along the lines:
namespace { // to keep things private as possible boost::thread_specific_ptr<mytype> foo_p; };
I think using an anonymous namespace makes no sense here as the ipp file is included in more than one translation unit (you would produce more than one instances of foo_p). Stefan
Hartmut Kaiser wrote:
Could you provide a patch?
I can ... a little static helper class is very helpful in this situation (I posted it some time ago to the list): //////////////////////////////////////// // class template static_ //////////////////////////////////////// template <typename T, typename OwnerT = void> struct static_ { T* operator -> () const { return &member; } static T* get() { return &member; } private: static T member; }; template <typename T, typename OwnerT> T static_<T, OwnerT>::member; /////////////// This way you can declare a global static instance of a type from anywhere in the code, i.e. in header files, because templates show a different behaviour for static members. The code could look like typedef boost::thread_specific_ptr<ptr_t> tld_t; static_<tld_t, X> tld_helper_static; tld_t tld_helper = tld_helper_static.get(); if (!tld_helper.get()) tld_helper.reset(new ptr_t); ptr_t &helper = *tld_helper; where X should denote any unique type (e.g. a private typedef) to distinguish this static instance of tld_t from other static instances of the same type. Cheers, Stefan
Stefan Slapeta wrote:
I can ... a little static helper class is very helpful in this situation (I posted it some time ago to the list):
//////////////////////////////////////// // class template static_ //////////////////////////////////////// template <typename T, typename OwnerT = void> struct static_ { T* operator -> () const { return &member; }
static T* get() { return &member; }
private: static T member; };
template <typename T, typename OwnerT> T static_<T, OwnerT>::member;
///////////////
This way you can declare a global static instance of a type from anywhere in the code, i.e. in header files, because templates show a different behaviour for static members.
This is a really nice trick! However a question: Is it guaranteed by the standard, that the ctor of the "member" is beeing called at program startup? Perhaps thread_specific_ptr itself should be implemented by means of this idiom then? Roland
On Wed, Jan 19, 2005 at 07:31:54PM +0100, Roland Schwarz wrote:
Stefan Slapeta wrote:
I can ... a little static helper class is very helpful in this situation (I posted it some time ago to the list):
//////////////////////////////////////// // class template static_ //////////////////////////////////////// template <typename T, typename OwnerT = void> struct static_ { T* operator -> () const { return &member; }
static T* get() { return &member; }
private: static T member; };
template <typename T, typename OwnerT> T static_<T, OwnerT>::member;
///////////////
This way you can declare a global static instance of a type from anywhere in the code, i.e. in header files, because templates show a different behaviour for static members.
This is a really nice trick! However a question: Is it guaranteed by the standard, that the ctor of the "member" is beeing called at program startup?
No. Unless the member is instantiated it will not be initialised. Since whether it is instantiated depends on whether you use it, which might depend on whether you've got testing/loggin/debugging code turned on, to guarantee that the member is initialised even when not used you might have to explicitly instantiate it for each specilalisation: template const int static_<int>::member = 0; template const bool static_<bool>::member = true;
Perhaps thread_specific_ptr itself should be implemented by means of this idiom then?
For that case it might not matter that the member isn't initialised unless used, but if you're trying to guarantee some side-effect of its construction always happens I think you need to explicitly instantiate. jon -- "That invisible hand of Adam Smith's seems to offer an extended middle finger to an awful lot of people." - George Carlin
Jonathan Wakely wrote:
No. Unless the member is instantiated it will not be initialised. Since whether it is instantiated depends on whether you use it, which might depend on whether you've got testing/loggin/debugging code turned on, to guarantee that the member is initialised even when not used you might have to explicitly instantiate it for each specilalisation:
template const int static_<int>::member = 0; template const bool static_<bool>::member = true;
If you ever access the member by calling static_<A>::get() in some piece of _instantiated code_ it's guaranteed that static_<A>::member is going to be instantiated (at program startup). Of course, this is not the case for e.g. function templates that are never instantiated or for inline functions that are not actually called. However, I think it's rather a feature than a bug not to instantiate a static_<A> member if the code is never executed. Stefan
On Thu, Jan 20, 2005 at 02:11:35PM +0100, Stefan Slapeta wrote:
Jonathan Wakely wrote:
No. Unless the member is instantiated it will not be initialised. Since whether it is instantiated depends on whether you use it, which might depend on whether you've got testing/loggin/debugging code turned on, to guarantee that the member is initialised even when not used you might have to explicitly instantiate it for each specilalisation:
template const int static_<int>::member = 0; template const bool static_<bool>::member = true;
If you ever access the member by calling static_<A>::get() in some piece of _instantiated code_ it's guaranteed that static_<A>::member is going to be instantiated (at program startup).
Yeah. I guess I was talking more about the general case, not your specific static_ class. For a more complicated class you could instantiate the class but that static member will not get instantiated unless used. For static_ it's pretty hard to instantiate it and not cause instantiation of the member too :-) For more complex code, where member might only get used in a debug build, you can't rely on it being instantiated in other builds.
Of course, this is not the case for e.g. function templates that are never instantiated or for inline functions that are not actually called. However, I think it's rather a feature than a bug not to instantiate a static_<A> member if the code is never executed.
Yes, I agree. I was just pointing out that behaviour and that you can't rely on static members of templates always being instantiated. If I've just confused the issue I apologise, for anyone unclear what I mean the attached file shows that although A<int> and A<char> are both instantiated, only A<int>::member is instantiated. jon
Stefan Slapeta wrote: [...]
The broader question is: Can local static objects ever be made thread safe? Or should they be avoided at all in MT aware code?
1) No 2) Yes!!
Quite the opposite is true. regards, alexander. -- "Other courts have reached the same conclusion: software is sold and not licensed." -- UNITED STATES DISTRICT COURT CENTRAL DISTRICT OF CALIFORNIA
Alexander Terekhov wrote:
Stefan Slapeta wrote: [...]
The broader question is: Can local static objects ever be made thread safe? Or should they be avoided at all in MT aware code?
1) No 2) Yes!!
Quite the opposite is true.
Can anyone shed some light on this? Is it a joke? Roland
Stefan Slapeta wrote:
Roland Schwarz wrote:
[...]
The broader question is: Can local static objects ever be made thread safe? Or should they be avoided at all in MT aware code?
1) No 2) Yes!!
Local static objects can be made safe by the compiler. The new cxx64 API requires it. They can also be made safe by using one of the following two approaches (pseudocode): 1. mutex m = MUTEX_INITIALIZER; void f() { mutex_lock(&m); static X x; mutex_unlock(&m); // use x } 2. X & instance() { static X x; return x; } void f() { call_once( instance ); X /*const*/ & x = instance(); // use x }
Peter Dimov wrote:
Local static objects can be made safe by the compiler. The new cxx64 API requires it.
Interesting, I didn't know that!
They can also be made safe by using one of the following two approaches (pseudocode):
[...] Sorry, (my) misinterpretation. I meant that you can not assume that a local static variable is synchronized by the compiler because this is not required by the standard. Stefan
Peter Dimov wrote: [...]
Local static objects can be made safe by the compiler. The new cxx64 API requires it.
Too bad that new cxx64 ABI couldn't mandate a standard #pragma (or something like that) to turn it on/off ("you should not pay for what you don't use" motto, you know). BTW, "__thread" extension aside for a moment (I guess it would fit best for boost::spirit), DCCI http://groups.google.de/groups?selm=415BD983.E2DA2114%40web.de is also an option for lazy inits (although its non-blocking strength gets totally eliminated in the case of blocking "new lazy()"). regards, alexander. -- "Static linking creates a derivative work through textual copying" -- GNU School of Law (aka GNU Church)
Alexander Terekhov wrote:
Peter Dimov wrote: [...]
Local static objects can be made safe by the compiler. The new cxx64 API requires it.
Too bad that new cxx64 ABI couldn't mandate a standard #pragma (or something like that) to turn it on/off ("you should not pay for what you don't use" motto, you know).
I actually think that not offering a way to turn it off is somewhat better, long term. Without an option to turn it off, compiler writers are forced to optimize local statics (using DCSI-MBR/DCCI/whatever).
"Peter Dimov" <pdimov@mmltd.net> writes:
Alexander Terekhov wrote:
Peter Dimov wrote: [...]
Local static objects can be made safe by the compiler. The new cxx64 API requires it.
Too bad that new cxx64 ABI couldn't mandate a standard #pragma (or something like that) to turn it on/off ("you should not pay for what you don't use" motto, you know).
I actually think that not offering a way to turn it off is somewhat better, long term. Without an option to turn it off, compiler writers are forced to optimize local statics (using DCSI-MBR/DCCI/whatever).
Any compiler writer is free to offer a way to turn it off as an extension. They're also free to not use those optimizations. -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
stas garifulin wrote:
I'm using vc71 with multi-threaded runtime. Sometimes then boost::spirit code is accessed from different threads simultaneously i have access violation at boost/spirit/core/non_terminal/impl/grammar.ipp (255):
# ifdef BOOST_SPIRIT_THREADSAFE static boost::thread_specific_ptr<ptr_t> tld_helper; // <-- Here if (!tld_helper.get()) tld_helper.reset(new ptr_t); ptr_t &helper = *tld_helper; # else
Possible same problem, local static variables: boost-1_32\boost\spirit\core\non_terminal\impl\object_with_id.ipp(130,133). boost-1_32\boost\spirit\core\primitives\impl\numerics.ipp(516) boost-1_32\boost\spirit\debug\impl\parser_names.ipp(420) boost-1_32\boost\spirit\utility\impl\chset_operators(511, 521) boost-1_32\boost\spirit\utility\impl\escape_char.ipp(192) boost-1_32\boost\spirit\phoenix\closures.hpp(404). Debugging and tree_to_xml doesn't work corectly in multi-threaded enviroment. Its use static ints for storing intendation info that should be moved to tls too.
participants (8)
-
Alexander Terekhov -
David Abrahams -
Hartmut Kaiser -
Jonathan Wakely -
Peter Dimov -
Roland Schwarz -
stas garifulin -
Stefan Slapeta