[thread] Race condition(?) on tss_data
Hello,
I'm using an AMD64 X2 and sometimes the program fails with the
debugger showing me this call stack:
> basic.exe!`anonymous namespace'::get_slots(bool alloc=false) Line
108 + 0x5 C++
basic.exe!boost::detail::tss::get() Line 164 + 0x7 C++
basic.exe!boost::thread_specific_ptr<boost::weak_ptr<boost::spirit::impl::grammar_helper<boost::spirit::grammar<smtp_client::impl::grammars::greeting_response,boost::spirit::parser_context<boost::spirit::nil_t>
>,smtp_client::impl::grammars::greeting_response,boost::spirit::scanner<char
*,boost::spirit::scanner_policies<boost::spirit::iteration_policy,boost::spirit::match_policy,boost::spirit::action_policy>
> > > >::get() Line 89 + 0x16 C++
basic.exe!boost::spirit::impl::get_definition<smtp_client::impl::grammars::greeting_response,boost::spirit::parser_context<boost::spirit::nil_t>,boost::spirit::scanner<char
*,boost::spirit::scanner_policies<boost::spirit::iteration_policy,boost::spirit::match_policy,boost::spirit::action_policy>
> >(const boost::spirit::grammar<smtp_client::impl::grammars::greeting_response,boost::spirit::parser_context<boost::spirit::nil_t>
> * self=0x005a6664) Line 226 + 0xa C++
basic.exe!boost::spirit::impl::grammar_parser_parse<0,smtp_client::impl::grammars::greeting_response,boost::spirit::parser_context<boost::spirit::nil_t>,boost::spirit::scanner<char
*,boost::spirit::scanner_policies<boost::spirit::iteration_policy,boost::spirit::match_policy,boost::spirit::action_policy>
> >(const boost::spirit::grammar<smtp_client::impl::grammars::greeting_response,boost::spirit::parser_context<boost::spirit::nil_t>
> * self=0x005a6664, const boost::spirit::scanner<char
*,boost::spirit::scanner_policies<boost::spirit::iteration_policy,boost::spirit::match_policy,boost::spirit::action_policy>
> & scan={...}) Line 279 + 0x9 C++
basic.exe!boost::spirit::grammar<smtp_client::impl::grammars::greeting_response,boost::spirit::parser_context<boost::spirit::nil_t>
>::parse_main<boost::spirit::scanner<char
*,boost::spirit::scanner_policies<boost::spirit::iteration_policy,boost::spirit::match_policy,boost::spirit::action_policy>
> >() Line 53 + 0x1f C++
basic.exe!boost::spirit::grammar<smtp_client::impl::grammars::greeting_response,boost::spirit::parser_context<boost::spirit::nil_t>
>::parse<boost::spirit::scanner<char
*,boost::spirit::scanner_policies<boost::spirit::iteration_policy,boost::spirit::match_policy,boost::spirit::action_policy>
> >() Line 63 + 0x42 C++
basic.exe!boost::spirit::parse<char
*,smtp_client::impl::grammars::greeting_response>(char * const &
first_=0x006e4374, char * const & last=0x006e439a, const
boost::spirit::parser<smtp_client::impl::grammars::greeting_response>
& p={...}) Line 30 C++
basic.exe!smtp_client::impl::detail::initialize_smtp_connection::match<char
*,smtp_client::impl::grammars::greeting_response>() Line 34 +
0x15 C++
basic.exe!smtp_client::impl::detail::initialize_smtp_connection::handle_greetings_read(boost::asio::error
& error={...}, unsigned int bytes_received=38) Line 61 + 0x2b C++
basic.exe!boost::_mfi::mf2<void,smtp_client::impl::detail::initialize_smtp_connection,boost::asio::error
&,unsigned int>::operator()(smtp_client::impl::detail::initialize_smtp_connection
* p=0x006e42c8, boost::asio::error & a1={...}, unsigned int a2=38)
Line 274 + 0x12 C++
basic.exe!boost::_bi::list3<boost::_bi::value<smtp_client::impl::detail::initialize_smtp_connection
*>,boost::arg<1>,boost::arg<2>
>::operator()<boost::_mfi::mf2<void,smtp_client::impl::detail::initialize_smtp_connection,boost::asio::error
&,unsigned int>,boost::_bi::list2<boost::asio::error &,unsigned int &>
>() Line 348 C++
basic.exe!boost::_bi::bind_t<void,boost::_mfi::mf2<void,smtp_client::impl::detail::initialize_smtp_connection,boost::asio::error
&,unsigned int>,boost::_bi::list3<boost::_bi::value<smtp_client::impl::detail::initialize_smtp_connection
*>,boost::arg<1>,boost::arg<2> >
>::operator()<boost::asio::error,unsigned int>() Line 62 C++
basic.exe!boost::detail::function::void_function_obj_invoker2<boost::_bi::bind_t<void,boost::_mfi::mf2<void,smtp_client::impl::detail::initialize_smtp_connection,boost::asio::error
&,unsigned int>,boost::_bi::list3<boost::_bi::value<smtp_client::impl::detail::initialize_smtp_connection
*>,boost::arg<1>,boost::arg<2> > >,void,boost::asio::error,unsigned
int>::invoke(boost::detail::function::function_buffer &
function_obj_ptr={...}, boost::asio::error a0={...}, unsigned int
a1=38) Line 146 C++
basic.exe!boost::function2<void,boost::asio::error,unsigned
int,_STL::allocator<void> >::operator()(boost::asio::error a0={...},
unsigned int a1=38) Line 682 + 0x2d C++
basic.exe!smtp_client::smtp_connection::handle_read(boost::asio::error
& error={...}, unsigned int bytes_readed=38, boost::function<void
__cdecl(boost::asio::error,unsigned int),_STL::allocator<void> >
handler={...}) Line 48 C++
basic.exe!boost::_mfi::mf3<void,smtp_client::smtp_connection,boost::asio::error
&,unsigned int,boost::function<void
__cdecl(boost::asio::error,unsigned int),_STL::allocator<void> >
>::operator()(smtp_client::smtp_connection * p=0x006d85b0,
boost::asio::error & a1={...}, unsigned int a2=38,
boost::function<void __cdecl(boost::asio::error,unsigned
int),_STL::allocator<void> > a3={...}) Line 384 + 0x24 C++
basic.exe!boost::_bi::list4<boost::_bi::value<smtp_client::smtp_connection
*>,boost::arg<1>,boost::arg<2>,boost::_bi::value<boost::function<void
__cdecl(boost::asio::error,unsigned int),_STL::allocator<void> > >
>::operator()<boost::_mfi::mf3<void,smtp_client::smtp_connection,boost::asio::error
&,unsigned int,boost::function<void
__cdecl(boost::asio::error,unsigned int),_STL::allocator<void> >
>,boost::_bi::list2<boost::asio::error &,unsigned int &> >() Line
413 C++
basic.exe!boost::_bi::bind_t<void,boost::_mfi::mf3<void,smtp_client::smtp_connection,boost::asio::error
&,unsigned int,boost::function<void
__cdecl(boost::asio::error,unsigned int),_STL::allocator<void> >
>,boost::_bi::list4<boost::_bi::value<smtp_client::smtp_connection
*>,boost::arg<1>,boost::arg<2>,boost::_bi::value<boost::function<void
__cdecl(boost::asio::error,unsigned int),_STL::allocator<void> > > >
>::operator()<boost::asio::error,unsigned int>() Line 62 C++
basic.exe!boost::asio::detail::win_iocp_socket_service<_STL::allocator<void>
>::receive_operation<boost::_bi::bind_t<void,boost::_mfi::mf3<void,smtp_client::smtp_connection,boost::asio::error
&,unsigned int,boost::function<void
__cdecl(boost::asio::error,unsigned int),_STL::allocator<void> >
>,boost::_bi::list4<boost::_bi::value<smtp_client::smtp_connection
*>,boost::arg<1>,boost::arg<2>,boost::_bi::value<boost::function<void
__cdecl(boost::asio::error,unsigned int),_STL::allocator<void> > > > >
>::do_completion_impl(boost::asio::detail::win_iocp_operation *
op=0x006e45a8, unsigned long last_error=0, unsigned int
bytes_transferred=38) Line 562 C++
basic.exe!boost::asio::detail::win_iocp_operation::do_completion()
Line 59 + 0x14
basic.exe!boost::asio::detail::win_iocp_demuxer_service::run() Line 104
basic.exe!boost::asio::demuxer_service<_STL::allocator<void>
>::run() Line 87
basic.exe!boost::asio::basic_demuxer<boost::asio::demuxer_service<_STL::allocator<void>
> >::run() Line 107
basic.exe!_main() Line 62
basic.exe!mainCRTStartup() Line 259 + 0x19 C
kernel32.dll!77e523cd()
As you can see, the problem is within get_slots, exactly on this line:
#if defined(BOOST_HAS_WINTHREADS)
slots = static_cast<tss_slots*>(
TlsGetValue(tss_data->native_key));
The disassembler shows me:
#if defined(BOOST_HAS_WINTHREADS)
slots = static_cast<tss_slots*>(
TlsGetValue(tss_data->native_key));
004DE3D9 mov eax,dword ptr [tss_data (5A8608h)]
004DE3DE mov ecx,dword ptr [eax+20h]
004DE3E1 push ecx
004DE3E2 call dword ptr [__imp__TlsGetValue@4 (5AB3B8h)]
And failing on the 004DE3DE with an access to 0x00000020. Which means
that it is reading tss_data as being 0.
This is being executed inside a parsing of a spirit grammar.
A static grammar that's accessed by two threads (on a dual core CPU).
It always strikes on the last "job".
I'm using asio and create 100 sockets, connect them to a SMTP and
start receiving and sending SMTP commands (using spirit to parse the
responses).
Everything is done asynchronously and the demuxer is executed by two threads.
I'm not very sure why tss_data is being seen as 0, and more strangely
in the last message. But prior any returning thread. (The main joins
the second thread, but is always the main thread that hits this).
Any help would be greatly appreciated,
Thanks in advance,
--
Felipe Magno de Almeida
Felipe Magno de Almeida wrote:
I'm using an AMD64 X2 and sometimes the program fails with the debugger showing me this call stack: [ snip ]
Unfortunately at the moment none of the developers seems to have access to a 64 bit machine. I will try to improve on this situation, but have to ask for patience until we have access. Regards Roland
On 4/7/06, Roland Schwarz <roland.schwarz@chello.at> wrote:
Felipe Magno de Almeida wrote:
I'm using an AMD64 X2 and sometimes the program fails with the debugger showing me this call stack: [ snip ]
Unfortunately at the moment none of the developers seems to have access to a 64 bit machine. I will try to improve on this situation, but have to ask for patience until we have access.
I forgot to mention! I'm compiling to x86. It seems the problem is due to boost::thread_specific_ptr<ptr_t> being a local static variable in get_definition on grammar.ipp in spirit code. line: 224 in grammar.ipp # ifdef BOOST_SPIRIT_THREADSAFE static boost::thread_specific_ptr<ptr_t> tld_helper; if (!tld_helper.get()) tld_helper.reset(new ptr_t); ptr_t &helper = *tld_helper; # else The .get member function is called before the tld_helper is fully constructed (and, as a consequence, the tss_data in tss.cpp isnt initialized too. So in line 114 - tss.cpp - ::get_slots slots = static_cast<tss_slots*>( TlsGetValue(tss_data->native_key)); tss_data isnt initialized yet. I think the only relevance is that it is a dual core. Which makes two parse's functions being called concurrently on a local static. The pretty bad workaround I made was adding boost::call_once(&init_tss_data, tss_data_once); as the first line to get_slots. But the real problem is that local statics arent thread safe, and a member-function is being called before the object being fully constructed. Unfortunately I dont have more time to invest coding any better solution right now. Nor understanding boost.thread/boost.spirit internals. It seems to me that thread_specific_ptr has a broken interface and documentation, since it doesnt address the local static thread-safety issue. Which leads users to use it erroneously. I dont know now if I should move the thread to spirit, as a spirit thread-safety bug. What do you think?
Regards Roland
Thanks, -- Felipe Magno de Almeida
Felipe Magno de Almeida wrote:
It seems to me that thread_specific_ptr has a broken interface and documentation, since it doesnt address the local static thread-safety issue. Which leads users to use it erroneously.
Whether local statics are thread-safe is implementation dependent, and is not limited to thread_specific_ptr. _Any_ dynamically initialized local static can lead to the failure you described.
On 4/7/06, Peter Dimov <pdimov@mmltd.net> wrote:
Felipe Magno de Almeida wrote:
It seems to me that thread_specific_ptr has a broken interface and documentation, since it doesnt address the local static thread-safety issue. Which leads users to use it erroneously.
Whether local statics are thread-safe is implementation dependent, and is not limited to thread_specific_ptr. _Any_ dynamically initialized local static can lead to the failure you described.
Surely, and that's why it is wrong to use it in portable code that way. And, IMO, the interface of thread_specific_ptr leads to believe that it is safe to use it that way. -- Felipe Magno de Almeida
Felipe Magno de Almeida wrote:
And, IMO, the interface of thread_specific_ptr leads to believe that it is safe to use it that way.
What specifically made you believe so? Thread safety can only be stated after an object has been created. And the thread_specific_ptr is no exception. To get platform independent behavior the ptr has to be declared globally. Roland
On 4/7/06, Roland Schwarz <roland.schwarz@chello.at> wrote: [snipped]
To get platform independent behavior the ptr has to be declared globally.
The problem is that it isnt possible when a ptr must be used in a free template function and the pointer is parameterized on the template function parameters. That was the spirit case. There isnt any solution from the boost.thread for this. Which, IMO, it leads to believe that thread_specific_ptr is the solution. After all, it is the only public solution for tss in boost.thread.
Roland
best regards, -- Felipe Magno de Almeida
Felipe Magno de Almeida wrote:
The problem is that it isnt possible when a ptr must be used in a free template function and the pointer is parameterized on the template function parameters. That was the spirit case. There isnt any solution from the boost.thread for this. Which, IMO, it leads to believe that thread_specific_ptr is the solution. After all, it is the only public solution for tss in boost.thread.
I did a liitle further research on the problem. In the function void foo() { static testclass bar; } the constructor call must be somehow conditioned on a variable (which normally is invisible to user code and automaticall compiler generated). This variable _is_ a shared resource and as such _must_ be protected! So if we had a statically initializeable mutex, we would say: void foo() { static mutex mtx = STATIC_MUTEX_INITIALIZER; scoped_lock lk(mtx); static testclass bar; lk.unlock(); } Since there is no static_mutex in the boost thread lib yet, one might use either boost/regex/pending/static_mutex.hpp or the following, which only uses boost::thread : namespace { void dont_clean(boost::mutex**) { }; boost::thread_specific_ptr<boost::mutex*> mutex_ptr(dont_clean); void mutex_init() { boost::mutex* pm = new boost::mutex; *(mutex_ptr.get()) = pm; } } void foo() { static boost::mutex* bar_mutex = 0; static boost::once_flag bar_once = BOOST_ONCE_INIT; mutex_ptr.reset(&bar_mutex); boost::call_once(&mutex_init, bar_once); boost::mutex::scoped_lock bar_lock(*bar_mutex); static testclass bar; bar_lock.unlock(); .... other code } There is a trade of currently between the both solutions: When the regex solution falls back to boost::thread based implementation there is only one global mutex beeing used for all objects of static_mutex. This is not the case with my solution. However mine leaves back a small memory leak for each. I definitely think boost thread should provide a static initializeable mutex, and this already is on the todo list. Altough the cleanup first has to be fixed. After all I think, spirit should implement one of the above in grammar.ipp, since both fix the outstanding bug. Roland
On 10/4/06, Roland Schwarz <roland.schwarz@chello.at> wrote:
Felipe Magno de Almeida wrote:
The problem is that it isnt possible when a ptr must be used in a free template function and the pointer is parameterized on the template function parameters. That was the spirit case. There isnt any solution from the boost.thread for this. Which, IMO, it leads to believe that thread_specific_ptr is the solution. After all, it is the only public solution for tss in boost.thread.
I did a liitle further research on the problem.
Thanks, looks like a nice solution.
[snipped]
Roland
Best regards, -- Felipe Magno de Almeida
"Felipe Magno de Almeida" <felipe.m.almeida@gmail.com> writes:
I dont know now if I should move the thread to spirit, as a spirit thread-safety bug. What do you think?
As Peter pointed out, local statics are not necessarily thread safe. Therefore this is a bug in spirit. Anthony -- Anthony Williams Software Developer Just Software Solutions Ltd http://www.justsoftwaresolutions.co.uk
participants (4)
-
Anthony Williams -
Felipe Magno de Almeida -
Peter Dimov -
Roland Schwarz