[exception] RFC: patch for printing stacktraces during exceptions.
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. 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. 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. The patch and a simple example are available here: http://wakko666.fedorapeople.org/boost/ ---Brett.
On 02/24/12 08:57, Brett Lentz wrote: [snip]
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. [snip] Finally, any other comments about this patch are welcome. I have attached both the patch, and some example code that will demonstrate the functionality.
The patch and a simple example are available here: http://wakko666.fedorapeople.org/boost/
---Brett. [snip]
Hi Brett, I tried to view in my firefox browser the following: http://wakko666.fedorapeople.org/boost/phusion_stacktrace.patch However, the response I got was: You don't have permission to access /boost/phusion_stacktrace.patch on this server. Could you make it readable to everyone? ---Larry
On 24/02/12 09:18 -0600, Larry Evans wrote:
On 02/24/12 08:57, Brett Lentz wrote: [snip]
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. [snip] Finally, any other comments about this patch are welcome. I have attached both the patch, and some example code that will demonstrate the functionality.
The patch and a simple example are available here: http://wakko666.fedorapeople.org/boost/
---Brett. [snip]
Hi Brett,
I tried to view in my firefox browser the following:
http://wakko666.fedorapeople.org/boost/phusion_stacktrace.patch
However, the response I got was:
You don't have permission to access /boost/phusion_stacktrace.patch on this server.
Could you make it readable to everyone?
---Larry
Done! Apologies for not checking that before sending the mail. ---Brett.
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
On Sat, Feb 25, 2012 at 3:41 AM, Vicente J. Botet Escriba < vicente.botet@wanadoo.fr> wrote:
Le 24/02/12 15:57, Brett Lentz a écrit :
[...] This has nothing to do with the technical aspects of the patch: AFAIK, "tracable" is not an English word (e.g., no entry in English wiktionary); I believe it should be "traceable". ...unless the intention is to make up a new word...which I wouldn't recommend. - Jeff
On 25/02/12 12:41 +0100, Vicente J. Botet Escriba wrote:
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?
I can do that or, as you mention later in your reply, I can use the spin lock implementation that already exists in boost.
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.
My personal goal is to get Passenger to the point where it can compile against the vanilla Boost libraries with minimal or no loss in functionality. This is a prerequisite for its inclusion into Fedora and is very useful for a few different reasons. Passenger is something a few different people have been trying to get into Fedora for quite a while now. I'm just trying to help the process along. (For the full history, see: https://bugzilla.redhat.com/show_bug.cgi?id=470696) I believe that the changes that Passenger currently ships in their bundled version of Boost are useful and generally applicable. Other users of Boost can benefit from the inclusion of features like fully traceable exceptions. There is a little bit of documentation in boost/exception/backtrace.hpp, but I'll see about getting some test cases created.
* I don't see any changes on windows files. Have you an implementation for windows?
I do not have a Windows implementation. That platform isn't the Passenger folks' focus, nor is it mine. If this is an issue that will prevent this feature from being included, I can see about adding Windows support to the patch.
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 :(
I'm open to suggestions on how you think it should be implemented. Passenger is highly multi-threaded server software, so that's the focus. If we need to make a change to make this feature more generally useful, it's something we can consider.
* 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?
The point of doing it this was was to have Boost.Thread throw exceptions that are derived from thread_exception. Doing it another way doesn't provide the same ability to trace the full length of the stack.
* 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.
I'll do that.
* There is already a spin lock implementation in Boost detail. Have you see if it responds to your needs?
This is something I will investigate. The current page is lifted from the Passenger code as closely to how it exists there. Our first goal was just to get a working patch that can be applied to demonstrate the functionality. Now that we've got that, I want to start figuring out what issues will prevent inclusion into Boost.
* I propose you to make a concrete proposal for the macros LIKELY and UNLIKELY to Boost.Config.
I'll do that. Being that this gets into compiler-specific and/or platform-specific territory, can you or anyone else comment on what minimum platforms need to be supported for inclusion?
* system_calls seems to me an independent thing of backtraces and tracable exceptions and quite dependant on posix interface.
I'll take a closer look at that and separate it out, if possible.
* 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?
Passenger is using the stock MIT License which appears to be what the Boost License is an extension of. I can ping the Passenger guys to see if the additional text in the second paragraph of the Boost License is acceptable to them. However, in my non-legal opinion, these licenses should be compatible as they're substantially the same text.
* 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
I will do that.
HTH,
Thanks so much for the comments. I'll be working on making the changes you suggest. Hopefully I'll have a new patch set to post to the list soon.
Vicente
---Brett.
On 25/02/12 12:41 +0100, Vicente J. Botet Escriba wrote:
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?
I can do that or, as you mention later in your reply, I can use the spin lock implementation that already exists in boost.
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.
My personal goal is to get Passenger to the point where it can compile against the vanilla Boost libraries with minimal or no loss in functionality. This is a prerequisite for its inclusion into Fedora and is very useful for a few different reasons. Passenger is something a few different people have been trying to get into Fedora for quite a while now. I'm just trying to help the process along. (For the full history, see: https://bugzilla.redhat.com/show_bug.cgi?id=470696)
I believe that the changes that Passenger currently ships in their bundled version of Boost are useful and generally applicable. Other users of Boost can benefit from the inclusion of features like fully traceable exceptions. I have no doubt of the utility of a backtrace library. The usual way is to propose a library for review.
There is a little bit of documentation in boost/exception/backtrace.hpp, but I'll see about getting some test cases created. I read them. More like that will help to understand user interface and
Le 27/02/12 16:41, Brett Lentz a écrit : the design.
* I don't see any changes on windows files. Have you an implementation for windows?
I do not have a Windows implementation. That platform isn't the Passenger folks' focus, nor is it mine. If this is an issue that will prevent this feature from being included, I can see about adding Windows support to the patch.
I don't know the effort to get the code portable on windows platform, but don't providing it is a show-stopper to his adoption.
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 :(
I'm open to suggestions on how you think it should be implemented. Passenger is highly multi-threaded server software, so that's the focus. If we need to make a change to make this feature more generally useful, it's something we can consider.
What I meant, is that will expect that the implementation of the file boost/exception/tracable_exception.hpp to be in the exception library. Anyway, it will be better if you can put all in a separated library, e.g? BackTrace
* Is it necessary that tracable_thread inherits from thread? What about adding a tracker that can be used on any thread?
Here again, it would help if you could define a tracable_thread or a tracker object, that doesn't need the modification of boost::thread. You could make traceable_thread a wrapper of boost::thread or use a thread specific Tracker object.
* 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?
The point of doing it this was was to have Boost.Thread throw exceptions that are derived from thread_exception. Doing it another way doesn't provide the same ability to trace the full length of the stack. I guess that any library will be free make its exceptions to inherit from traceable_exception, and this should not imply to lost the its current interface. All the thread_exception, must inherit from boost::system_error.
* There is already a spin lock implementation in Boost detail. Have you see if it responds to your needs?
This is something I will investigate. The current page is lifted from the Passenger code as closely to how it exists there. Our first goal was just to get a working patch that can be applied to demonstrate the functionality. Now that we've got that, I want to start figuring out what issues will prevent inclusion into Boost.
* I propose you to make a concrete proposal for the macros LIKELY and UNLIKELY to Boost.Config.
I'll do that. Being that this gets into compiler-specific and/or platform-specific territory, can you or anyone else comment on what minimum platforms need to be supported for inclusion? As this is an optimization feature, I think it will be enough to provide it for the compilers supporting it. The questions is if the proposed macros works for all the compiler proposing a similar feature.
* I don't know if the license is compatible with the Boost license. Could you use the Boost license?
Passenger is using the stock MIT License which appears to be what the Boost License is an extension of. I can ping the Passenger guys to see if the additional text in the second paragraph of the Boost License is acceptable to them. However, in my non-legal opinion, these licenses should be compatible as they're substantially the same text. AFAIK, every Boost file must contain the Boost license.
Thanks so much for the comments. I'll be working on making the changes you suggest. Hopefully I'll have a new patch set to post to the list soon.
Please, try to convert the patch in one or several separated libraries or specific patches for each library. It will much easier to get at least some of them accepted. If you don't reach to get them included into Boost, you could always add them as part of Passenger. Best, Vicente
I'd suggest checking out Boost.Backtrace: http://lists.boost.org/Archives/boost/2010/10/172303.php We use this at work and we've been very happy with it. On 2012.02.24 09.57, Brett Lentz wrote:
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.
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.
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.
The patch and a simple example are available here: http://wakko666.fedorapeople.org/boost/
---Brett.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Bryce Lelbach aka wash STE||AR Group, Center for Computation and Science, LSU -- boost-spirit.com stellar.cct.lsu.edu llvm.wiki.kernel.org
This patch adds code which is not under the Boost license. Would you be willing to use the Boost software license for this code? On 2012.02.24 09.57, Brett Lentz wrote:
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.
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.
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.
The patch and a simple example are available here: http://wakko666.fedorapeople.org/boost/
---Brett.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Bryce Lelbach aka wash STE||AR Group, Center for Computation and Science, LSU -- boost-spirit.com stellar.cct.lsu.edu llvm.wiki.kernel.org
I'm not sure why you've added your own spinlock to Boost with this patch. There's a perfectly good spinlock available in boost.smart_ptr. I'd suggest using this instead. On 2012.02.24 09.57, Brett Lentz wrote:
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.
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.
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.
The patch and a simple example are available here: http://wakko666.fedorapeople.org/boost/
---Brett.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Bryce Lelbach aka wash STE||AR Group, Center for Computation and Science, LSU -- boost-spirit.com stellar.cct.lsu.edu llvm.wiki.kernel.org
participants (5)
-
Brett Lentz -
Bryce Lelbach -
Jeffrey Lee Hellrung, Jr. -
Larry Evans -
Vicente J. Botet Escriba