#3848 closed Bugs (fixed)
Boost.Exception and transporting exceptions between threads
Reported by: | Owned by: | Emil Dotchevski | |
---|---|---|---|
Milestone: | Component: | exception | |
Version: | Boost 1.40.0 | Severity: | Problem |
Keywords: | exception thread safety | Cc: | nikkikom@… |
Description
The attached example program exc.cc crashes from time to time. I suspect that the problem is that detail::error_info_container_impl is not thread safe.
Here is gdb stack trace: Core was generated by `./exc'. Program terminated with signal 11, Segmentation fault.
#0 0x0000000000000000 in ?? () #1 0x000000000040532a in boost::exception_detail::refcount_ptr<boost::exception_detail::error_info_container>::release (this=0x2aaab4001e28) at include/boost/exception/exception.hpp:73 #2 0x0000000000405341 in ~refcount_ptr (this=0x2aaab4001e28)
at include/boost/exception/exception.hpp:28
#3 0x000000000040da36 in ~exception (this=0x2aaab4001e20)
at include/boost/exception/exception.hpp:255
#4 0x000000000040e2f7 in ~err (this=0x2aaab4001e20) at exc.cc:10 #5 0x000000000040e437 in ~clone_impl (this=0x2aaab4001e20)
at include/boost/exception/exception.hpp:368
#6 0x000000000040612f in boost::checked_delete<boost::exception_detail::clone_base const> (x=0x2aaab4001e50)
at include/boost/checked_delete.hpp:34
#7 0x00000000004074ff in boost::detail::sp_counted_impl_p<boost::exception_detail::clone_base const>::dispose (this=0x2aaab4000910) at include/boost/smart_ptr/detail/sp_counted_impl.hpp:78 #8 0x0000000000404706 in boost::detail::sp_counted_base::release (this=0x2aaab4000910)
at include/boost/smart_ptr/detail/sp_counted_base_gcc_x86.hpp:145
#9 0x0000000000404762 in ~shared_count (this=0x42002fe8)
at include/boost/smart_ptr/detail/shared_count.hpp:217
#10 0x0000000000404ae9 in ~shared_ptr (this=0x42002fe0)
at include/boost/smart_ptr/shared_ptr.hpp:169
#11 0x0000000000404c5e in ~exception_ptr (this=0x42002fd8)
at include/boost/exception_ptr.hpp:43
#12 0x000000000040ca4b in ~future (this=0x42002fd0) at exc.cc:14 #13 0x00000000004041f3 in consumer () at exc.cc:54 #14 0x0000000000404253 in consume () at exc.cc:61 #15 0x00000000004071b3 in boost::detail::thread_data<void (*)()>::run (this=0x6238c0)
at include/boost/thread/detail/thread.hpp:56
#16 0x00002b033252d33f in thread_proxy () from lib/libboost_thread.so.1.40.0 #17 0x00002b0333651fc7 in start_thread () from lib/libpthread.so.0 #18 0x00002b03331bd5ad in clone () from lib/libc.so.6 #19 0x0000000000000000 in ?? ()
Attachments (3)
Change History (11)
by , 13 years ago
comment:1 by , 13 years ago
Cc: | added |
---|
comment:2 by , 13 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
The exception object itself is NOT thread safe.
Try this instead:
.... boost::exception_ptr get_exception () const { boost::unique_lock<boost::mutex> lck (mux_); while (! ready_) cond_.wait (lck); return exc_; } .... future f; boost::thread thr (boost::bind (&producer, boost::ref (f))); if( boost::exception_ptr e=f.get_exception() ) rethrow_exception(e);
by , 13 years ago
by , 13 years ago
Attachment: | exc1_log.txt added |
---|
comment:3 by , 13 years ago
Resolution: | invalid |
---|---|
Status: | closed → reopened |
Please take a look at example a little bit closer.
I'm not transferring boost::exception object between threads, I'm transferring exception_ptr.
Boost.Exception docs says about thread safety:
- It is legal for multiple threads to hold exception_ptr references to the same exception object.
- It is illegal for multiple threads to modify the same exception_ptr object concurrently.
- While calling current_exception makes a copy of the current exception object, it is still possible for the two copies to share internal state. Therefore, in general it is not safe to call rethrow_exception concurrently to throw the same exception object into multiple threads.
Well, I do not modify same exception_ptr object concurrently. And I do not call rethrow_exception concurrently to throw the same exception object.
I guess the race condition happen when temporal boost::exception object destructor called in producer thread in the very same time when rethrow_exception called in consumer thread.
I applied the changes you suggested, the example still crashes. Please take a look at attached exc1.cc and exc1_log.txt.
BTW, it is possible to reproduce the very same bug with boost::promise/boost::future in 1.41+
void producer (boost::promise<void>& promise, boost::barrier& barrier) { barrier.wait (); promise.set_exception (boost::copy_exception (err () << err_info ("stub"))); } void consumer () { boost::barrier barrier (2); boost::promise<void> promise; boost::unique_future<void> future = promise.get_future (); boost::thread thr ( boost::bind (&producer, boost::ref (promise), boost::ref (barrier)) ); barrier.wait (); try { future.get (); } catch (err const& e) {} thr.join (); }
It should fail as well.
follow-up: 5 comment:4 by , 13 years ago
This simple patch would help:
--- boost.1.40/exception/info.hpp 2009-10-09 23:11:15.000000000 +0400 +++boost/exception/info.hpp 2010-01-25 14:50:03.000000000 +0300 @@ -10,6 +10,7 @@ #include <boost/exception/to_string_stub.hpp> #include <boost/exception/detail/error_info_impl.hpp> #include <boost/shared_ptr.hpp> +#include <boost/thread.hpp> #include <map> namespace @@ -121,19 +122,25 @@ error_info_map info_; mutable std::string diagnostic_info_str_; mutable int count_; + mutable mutex mux_; void add_ref() const { + unique_lock<mutex> lock (mux_); ++count_; } void release() const { + unique_lock<mutex> lock (mux_); if( !--count_ ) + { + lock.unlock (); delete this; } + } }; }
However the atomic count_ implementation would be much more efficient.
comment:5 by , 13 years ago
Resolution: | → invalid |
---|---|
Status: | reopened → closed |
Forget about boost::exception for a moment, think about using exception_ptr to transfer an exception of some user-defined type between threads. It is incorrect to assume that copies of the exception object don't have shared state, or that if they do they do it in a thread-safe manner, because in principle exceptions are thread-local objects (note that using shared state is the only way non-trivial exception types can provide no-throw copy constructor which the C++ standard requires.)
It is not possible for exception_ptr to help you solve this problem.
The solution is to join the thread before rethrowing the exception. See http://svn.boost.org/svn/boost/trunk/libs/exception/test/exception_ptr_test.cpp.
comment:6 by , 13 years ago
Resolution: | invalid |
---|---|
Status: | closed → reopened |
comment:7 by , 13 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Nikki, thanks for reporting this tricky bug. Should be fixed in trunk revision 59364. Your code is now incorporated in copy_exception_test.cpp.
comment:8 by , 13 years ago
Thank you.
Also I'm getting a lot of errors when translating latest svn boost with g++ 4.4.2 20091222 (Red Hat 4.4.2-20) (GCC) in с++0x mode.
g++ -std=c++0x -O6 -s -I. -I../boost/trunk -o t test.cc -lltdl In file included from ../boost/trunk/boost/exception_ptr.hpp:9, from ../boost/trunk/boost/exception/all.hpp:30, from ./service/error.h:6, from ./service/library.h:7, from test.cc:4: ../boost/trunk/boost/exception/detail/exception_ptr.hpp: In function ‘boost::exception_ptr boost::exception_detail::get_bad_alloc()’: ../boost/trunk/boost/exception/detail/exception_ptr.hpp:80: error: call of overloaded ‘copy_exception(const boost::exception_detail::bad_alloc_&)’ is ambiguous ../boost/trunk/boost/exception/detail/exception_ptr.hpp:38: note: candidates are: boost::exception_ptr boost::copy_exception(const T&) [with T = boost::exception_detail::bad_alloc_] /usr/lib/gcc/x86_64-redhat-linux/4.4.2/../../../../include/c++/4.4.2/exception_ptr.h:152: note: std::__exception_ptr::exception_ptr std::copy_exception(_Ex) [with _Ex = boost::exception_detail::bad_alloc_] ../boost/trunk/boost/exception/detail/exception_ptr.hpp: In function ‘boost::exception_ptr boost::exception_detail::current_exception_unknown_exception()’: ../boost/trunk/boost/exception/detail/exception_ptr.hpp:221: error: call of overloaded ‘copy_exception(boost::unknown_exception)’ is ambiguous ../boost/trunk/boost/exception/detail/exception_ptr.hpp:38: note: candidates are: boost::exception_ptr boost::copy_exception(const T&) [with T = boost::unknown_exception] /usr/lib/gcc/x86_64-redhat-linux/4.4.2/../../../../include/c++/4.4.2/exception_ptr.h:152: note: std::__exception_ptr::exception_ptr std::copy_exception(_Ex) [with _Ex = boost::unknown_exception] ../boost/trunk/boost/exception/detail/exception_ptr.hpp: In function ‘boost::exception_ptr boost::exception_detail::current_exception_unknown_boost_exception(const boost::exception&)’: ../boost/trunk/boost/exception/detail/exception_ptr.hpp:228: error: call of overloaded ‘copy_exception(boost::unknown_exception)’ is ambiguous etc... etc...
I would suggest the following patch, it works great:
--- boost/exception/detail/exception_ptr.hpp 2010-02-03 20:08:23.114427338 +0300 +++ /usr/local/include/boost/exception/detail/exception_ptr.hpp 2010-02-04 05:57:37.490551407 +0300 @@ -73,7 +73,7 @@ exception_ptr get_bad_alloc() { - static exception_ptr e = copy_exception( + static exception_ptr e = boost::copy_exception( bad_alloc_() << throw_function("boost::current_exception()") << throw_file(__FILE__) << @@ -209,23 +209,23 @@ current_exception_std_exception( T const & e1 ) { if( boost::exception const * e2 = get_boost_exception(&e1) ) - return copy_exception(current_exception_std_exception_wrapper<T>(e1,*e2)); + return boost::copy_exception(current_exception_std_exception_wrapper<T>(e1,*e2)); else - return copy_exception(current_exception_std_exception_wrapper<T>(e1)); + return boost::copy_exception(current_exception_std_exception_wrapper<T>(e1)); } inline exception_ptr current_exception_unknown_exception() { - return copy_exception(unknown_exception()); + return boost::copy_exception(unknown_exception()); } inline exception_ptr current_exception_unknown_boost_exception( boost::exception const & e ) { - return copy_exception(unknown_exception(e)); + return boost::copy_exception(unknown_exception(e)); } inline @@ -235,7 +235,7 @@ if( boost::exception const * be = get_boost_exception(&e) ) return current_exception_unknown_boost_exception(*be); else - return copy_exception(unknown_exception(e)); + return boost::copy_exception(unknown_exception(e)); } inline
Sorry for backtrace misformatting. Here is the right one: