Re: [boost] [Stacktrace] review

Subject: Re: [boost] [Stacktrace] review
From: Emil Dotchevski (emildotchevski_at_[hidden])
Date: 2016-12-16 14:32:35

On Fri, Dec 16, 2016 at 1:20 AM, Andrey Semashev <andrey.semashev_at_[hidden]>
wrote:

> On 12/16/16 03:00, Emil Dotchevski wrote:
>
>>
>> I am not proposing a dependency of throw_exception on Starktrace, but
>> removing a minimal amount of code from Stacktrace (e.g. something that
>> packs everything into a memory buffer) and putting it into
>> throw_exception.
>> I do think that logically, this code should be left in a separate header,
>> but it has to be written without any Boost dependencies. If that is not
>> possible, I agree that the integration is a bad idea.
>>
>
> I'm not sure what else Boost.Stacktrace does; it looks like it means
> moving the bulk of Stacktrace into the throw_exception submodule. I don't
> think that is a reasonable solution in terms of code management.
>

Maybe not. What I'm saying is that 1) automatic capture of stack trace is
very useful and 2) any possible integration into boost::throw_exception
requires careful consideration. Let's see the actual code before we say it
is not acceptable.

> Also, I'm not sure what APIs Stacktrace uses behind the scene, does it
> depend on additional third party libraries to obtain the backtrace?
>

I am not sure either, I'm assuming it needs only depend on system
libraries. Obviously if that's not the case the integration into
throw_exception is a no-go.

> the argument wouldn't be whether or not capturing the
>> stack trace is integrated into boost::throw_exception, but whether or not
>> it is enabled by default. Either way, the boost::exception class (defined
>> in boost/exception/exception.hpp) should provide a hook for that
>> integration in order to avoid coupling between boost::throw_exception and
>> Stacktrace.
>>
>
> The way I see it, boost::exception already provides everything needed,
> which is the ability to attach arbitrary data to the exception. I don't
> think adding more special case capability to it is a wise solution in the
> long term.
>

This can be said about __FILE__ and __LINE__ which BOOST_THROW_EXCEPTION
captures automatically. The reason why it's done is that 1) the cost in
terms of time and space is considered negligible and 2) it is potentially
critical for debugging.

The reason why it Stacktrace would need special handling rather than go
through the usual Boost Exception machinery is also the same as why there
is special handling (members in boost::exception): to avoid coupling with
the rest of the Boost Exception machinery, which is considerably heavier
and bigger than boost/exception/exception.hpp.

> The run-time hook can be tricky to implement, that is true, but in the
>>> simplest form it should be doable. The implementation details are of course
>>> discussable, but below is something more concrete to get things started.
>>>
>>> Introduce a process-wide singleton pointer, which is a pointer to the
>>> function that will augment the exception about to be thrown:
>>>
>>>   typedef void (*exception_augmenter)(boost::exception&);
>>>
>>
Consider dynamically loaded libraries. Also consider multiple libraries
wanting different hooks. A process-wide pointer is very difficult to do in
the general case, and I suspect it'd drag a lot more dependencies than
capturing the stacktrace would. Consider that boost::throw_exception must
support very very old compilers.

Besides, throwing exceptions and capturing stack trace are thread-local
operations, while global pointer would require synchronization with the
initialization code.

> The augmenter will be able to use Boost.Exception API to add any data to
>>> the exception.
>>>
>>> The pointer can be updated atomically. If it deems necessary, a
>>> thread-local and module-local copy can be maintained. The default value of
>>> null means no augmenter and throwing the exception works as
>>> BOOST_THROW_EXCEPTION currently does.
>>>
>>> The tricky part is to maintain the singleton. On Windows one can use a
>>> named semaphore to store the pointer as the semaphore state (see
>>> https://github.com/boostorg/sync/blob/develop/include/boost/
>>> sync/detail/waitable_timer.hpp#L121 for an example, Boost.Interprocess
>>> also has something similar). On POSIX systems it might be enough to just
>>> mark the pointer with default visibility. A solution with shared memory
>>> (shmget/shmat/etc.) also seems possible.
>>>
>>
Are you proposing to include <windows.h> in boost/throw_exception.hpp?
That's a no-go. Making it not header-only module? Also a no-go.

Emil