Boost C++ Libraries: Ticket #3848: Boost.Exception and transporting exceptions between threads

attachment set

Tue, 19 Jan 2010 17:55:22 GMT

attachment → exc.cc

cc set

Tue, 19 Jan 2010 18:09:43 GMT

cc nikkikom@… added

Sorry for backtrace misformatting. Here is the right one:

#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 ?? ()}}}

status changed; resolution set

Emil Dotchevski — Wed, 20 Jan 2010 03:36:58 GMT

status new → closed
resolution → invalid

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);

attachment set

Wed, 20 Jan 2010 12:00:06 GMT

attachment → exc1.cc

attachment set

Wed, 20 Jan 2010 12:00:21 GMT

attachment → exc1_log.txt

status changed; resolution deleted

Wed, 20 Jan 2010 12:15:27 GMT

status closed → reopened
resolution invalid

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.

Mon, 25 Jan 2010 13:57:15 GMT

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.

status changed; resolution set

Emil Dotchevski — Fri, 29 Jan 2010 17:11:36 GMT

status reopened → closed
resolution → invalid

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.

status changed; resolution deleted

anonymous — Fri, 29 Jan 2010 22:40:30 GMT

status closed → reopened
resolution invalid

status changed; resolution set

Emil Dotchevski — Sat, 30 Jan 2010 05:31:44 GMT

status reopened → closed
resolution → fixed

Nikki, thanks for reporting this tricky bug. Should be fixed in trunk revision 59364. Your code is now incorporated in copy_exception_test.cpp.

Thu, 04 Feb 2010 13:51:30 GMT

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