#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: