Re: [boost] [exception] RFC: patch for printing stacktraces during exceptions.

Subject: Re: [boost] [exception] RFC: patch for printing stacktraces during exceptions.
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2012-02-25 06:41:42

Le 24/02/12 15:57, Brett Lentz a écrit :
> Hi folks!
>
> It's been a few months since I last mailed the list. I've been working on
> bringing some forked changes from the Passenger project into Boost due 
> to their
> more general applicability.
>
> With some help, I've got a working patch against SVN trunk that 
> enables one of
> Passenger's more useful features: obtaining exception stacktraces all 
> the way
> through the stack.
>
> However, due to the size of the patch and some namespacing choices 
> made by the
> Phusion people, I'd like to get some feedback on this patch to ensure 
> it's
> suitable for being brought into Boost.
>
Hi,

I wanted just to note that this patch concerns the libraries 
Boost.Exception, Boost.System and Boost.Thread and that it is a library 
without a name.
Could it be possible to define these functionalities in independent 
libraries?

> First, there's some helper classes that Phusion has put in an "oxt" 
> namespace
> that I'm not really sure where they should be put or if suitable 
> analogues
> already exist within Boost. For the current version of the patch, 
> these files
> are simply put into 'boost/oxt' until I have a better location for them.
Which libraries are using it in your patch. Maybe you can put it on 
boost::detail::oxt?
>
> Second, the current patch uses boost::tracable_exception. I'd like to 
> confirm
> whether that is acceptable or whether I should move this to a different
> location in the boost namespace.
>
> Finally, any other comments about this patch are welcome. I have 
> attached both
> the patch, and some example code that will demonstrate the functionality.
>
>
I don't know what is your intention proposing this patch, but I don't 
think the patch in its actual form could be applied to Boost. Its is a 
big patch changing a lot of code on existing libraries without any test, 
neither documentation.

* Here there are the files contained on this patch.

NEW Index: boost/thread/tracable_thread.hpp
Index: boost/thread/detail/thread.hpp
Index: boost/thread/exceptions.hpp
NEW Index: boost/thread/dynamic_thread_group.hpp
NEW Index: boost/oxt/detail/spin_lock_portable.hpp
NEW Index: boost/oxt/detail/spin_lock_pthreads.hpp
NEW Index: boost/oxt/detail/spin_lock_gcc_x86.hpp
NEW Index: boost/oxt/detail/spin_lock_darwin.hpp
NEW Index: boost/oxt/spin_lock.hppIndex: boost/oxt/macros.hpp
NEW Index: boost/system/system_calls.hpp
NEW Index: boost/exception/tracable_exception.hpp
NEW Index: boost/exception/detail/backtrace_enabled.hpp
NEW Index: boost/exception/detail/backtrace_disabled.hpp
NEW Index: boost/exception/detail/tracable_exception_disabled.hpp
NEW Index: boost/exception/detail/tracable_exception_enabled.hpp
NEW Index: boost/exception/backtrace.hpp
Index: libs/thread/build/Jamfile.v2
Index: libs/thread/src/tss_null.cpp
NEW Index: libs/thread/src/pthread/backtrace.cpp
Index: libs/thread/src/pthread/once.cpp
NEW Index: libs/thread/src/pthread/tracable_thread.cpp
Index: libs/thread/src/pthread/thread.cpp
NEW Index: libs/thread/src/pthread/tracable_exception.cpp
Index: libs/system/build/Jamfile.v2
NEW Index: libs/system/src/system_calls.cpp

* I don't see any changes on windows files. Have you an implementation 
for windows?

Note that the patch contains the files

Index: boost/exception/tracable_exception.hpp

Index: libs/thread/src/pthread/tracable_thread.cpp

That is, the declaration is in Boost.Exception, while the implementation 
is in Boost.Thread :(

* Is it necessary that tracable_thread inherits from thread? What about 
adding a tracker that can be used on any thread?

* Why thread_exception don't inherits now from boost::system_error? This 
breaks the current exception interface. Is it absolutely needed that 
thread_exception inherits from tracable_exception to provide tracable 
exceptions?

* It seems to me that dynamic_thread_group could be a nice to have, but 
not required to have backtraces, so it should be proposed separately.

* There is already a spin lock implementation in Boost detail. Have you 
see if it responds to your needs?

* I propose you to make a concrete proposal for the macros LIKELY and 
UNLIKELY to Boost.Config.

* system_calls seems to me an independent thing of backtraces and 
tracable exceptions and quite dependant on posix interface.

* It is current usage on Boost to place the implementation details in 
the namespace detail (see all the functions prefixed by "_" and to 
prefix any macro with BOOST_LIBNAME_.

* I don't know if the license is compatible with the Boost license. 
Could you use the Boost license?

* Please could you make a ticket concerning the changes in

Index: libs/thread/src/tss_null.cpp

Index: libs/thread/src/pthread/once.cpp

Index: libs/thread/src/pthread/thread.cpp

HTH,

Vicente