`backward::SignalHandling` `crash_status` not resetting to `crash_status::running`
On Windows at least, there appears to be an edge case that only shows up when not linking against backward.cpp's singleton instance of backward::SignalHandling. Instead, in my code I was creating and destroying SignalHandling instances as needed. As a result, the reporter_thread_ exits shortly after starting, since crashed() returns crash_status::ending by the time that thread starts back up again in the new SignalHandling instance. Do you intend to support re-creating SignalHandling instances? If so, I have a potential fix below. It works in my testing, but only if at most one SignalHandling instance exists at a time (this doesn't make sense to support anyway).
diff --git a/backward.hpp b/backward.hpp
index 670aa45..632b372 100644
--- a/backward.hpp
+++ b/backward.hpp
@@ -4311,28 +4311,38 @@ private:
class SignalHandling {
public:
SignalHandling(const std::vector<int> & = std::vector<int>())
- : reporter_thread_([]() {
- /* We handle crashes in a utility thread:
- backward structures and some Windows functions called here
- need stack space, which we do not have when we encounter a
- stack overflow.
- To support reporting stack traces during a stack overflow,
- we create a utility thread at startup, which waits until a
- crash happens or the program exits normally. */
-
- {
- std::unique_lock<std::mutex> lk(mtx());
- cv().wait(lk, [] { return crashed() != crash_status::running; });
- }
- if (crashed() == crash_status::crashed) {
- handle_stacktrace(skip_recs());
- }
- {
- std::unique_lock<std::mutex> lk(mtx());
- crashed() = crash_status::ending;
- }
- cv().notify_one();
- }) {
+ {
+ // Reset crash status
+ {
+ std::unique_lock<std::mutex> lk(mtx());
+ crashed() = crash_status::running;
+ }
+
+ reporter_thread_ = std::thread([]() {
+ /* We handle crashes in a utility thread:
+ backward structures and some Windows functions called here
+ need stack space, which we do not have when we encounter a
+ stack overflow.
+ To support reporting stack traces during a stack overflow,
+ we create a utility thread at startup, which waits until a
+ crash happens or the program exits normally. */
+
+ crash_status status;
+ {
+ std::unique_lock<std::mutex> lk(mtx());
+ cv().wait(lk, [] { return crashed() != crash_status::running; });
+ status = crashed();
+ }
+ if (status == crash_status::crashed) {
+ handle_stacktrace(skip_recs());
+ }
+ {
+ std::unique_lock<std::mutex> lk(mtx());
+ crashed() = crash_status::ending;
+ }
+ cv().notify_one();
+ });
+
SetUnhandledExceptionFilter(crash_handler);
signal(SIGABRT, signal_handler);
Without much thoughts, I think this can be considered a bug? I don't think SignalHandling should inherently be impossible to cleanly destroy.
The code originally didn't bother using a thread. One reason what that std::thread didn't exist and so there wasn't an easy way to get portable threads.
I a way I made mistake accepting the code with std::thread because this breaks backward-cpp compatibility with C++98. On the other hand it might be time to accept that C++11 should be the new lowest version to support. I keep telling people to avoid breaking C++98 in pull requests, but now I realize the code probably doesn't even compile on it anymore.
And yes this also shows that I have not ran nor tested the code in a long time. I merely comment and accept pull requests. And I don't have any of the hardware backward-cpp can run on that people have contributed to over the years.
With this context, it hopefully helps you decide if this is indeed a bug fix. I will gladly merge a pull request. Maybe add a short comment in the code and API doc in the readme about the requirement of SignalHandling to never be instantiated more than once at a time.
(btw your diff is quite hard to read, I assume because of some indentation change) On Fri, May 9, 2025, 11:32 sbond75 @.***> wrote:
sbond75 created an issue (bombela/backward-cpp#351) https://github.com/bombela/backward-cpp/issues/351
On Windows at least, there appears to be an edge case that only shows up when not linking against backward.cpp's singleton instance of backward::SignalHandling. Instead, in my code I was creating and destroying SignalHandling instances as needed. As a result, the reporter_thread_ exits shortly after starting, since crashed() returns crash_status::ending by the time that thread starts back up again in the new SignalHandling instance. Do you intend to support re-creating SignalHandling instances? If so, I have a potential fix below. It works in my testing, but only if at most one SignalHandling instance exists at a time (this doesn't make sense to support anyway).
diff --git a/backward.hpp b/backward.hpp index 670aa45..632b372 100644--- a/backward.hpp+++ b/backward.hpp@@ -4311,28 +4311,38 @@ private: class SignalHandling { public: SignalHandling(const std::vector
& = std::vector ())- : reporter_thread_( {- /* We handle crashes in a utility thread:- backward structures and some Windows functions called here- need stack space, which we do not have when we encounter a- stack overflow.- To support reporting stack traces during a stack overflow,- we create a utility thread at startup, which waits until a- crash happens or the program exits normally. /-- {- std::unique_lockstd::mutex lk(mtx());- cv().wait(lk, [] { return crashed() != crash_status::running; });- }- if (crashed() == crash_status::crashed) {- handle_stacktrace(skip_recs());- }- {- std::unique_lockstd::mutex lk(mtx());- crashed() = crash_status::ending;- }- cv().notify_one();- }) {+ {+ // Reset crash status+ {+ std::unique_lockstd::mutex lk(mtx());+ crashed() = crash_status::running;+ }++ reporter_thread_ = std::thread( {+ / We handle crashes in a utility thread:+ backward structures and some Windows functions called here+ need stack space, which we do not have when we encounter a+ stack overflow.+ To support reporting stack traces during a stack overflow,+ we create a utility thread at startup, which waits until a+ crash happens or the program exits normally. */++ crash_status status;+ {+ std::unique_lockstd::mutex lk(mtx());+ cv().wait(lk, [] { return crashed() != crash_status::running; });+ status = crashed();+ }+ if (status == crash_status::crashed) {+ handle_stacktrace(skip_recs());+ }+ {+ std::unique_lockstd::mutex lk(mtx());+ crashed() = crash_status::ending;+ }+ cv().notify_one();+ });+ SetUnhandledExceptionFilter(crash_handler); signal(SIGABRT, signal_handler);— Reply to this email directly, view it on GitHub https://github.com/bombela/backward-cpp/issues/351, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABUZDCS63BM3AYZC545UML25TYDHAVCNFSM6AAAAAB4ZWETXGVHI2DSMVQWIX3LMV43ASLTON2WKOZTGA2TEOJQHEYTEMY . You are receiving this because you are subscribed to this thread.Message ID: @.***>