Skip to content

Conversation

ovaar
Copy link

@ovaar ovaar commented Aug 26, 2025

…ug due to CRT's SIGABRT handler behaviour

Scenario:

  1. On Windows, the first crash is handled via UnhandledExceptionFilter -> crash_handler(EXCEPTION_POINTERS*), which wakes the reporter thread to collect and print the stack trace.
  2. After printing, our handler calls abort(). In Debug CRT, abort raises SIGABRT and invokes our installed signal handler again.
  3. On this second entry, crash_handler(int) sets crashed() = crash_status::crashed and waits on the condition variable. But the reporter thread has already finished (set crashed() = ending) and exited, so no one flips the state anymore. The wait blocks forever, causing a deadlock that only reproduces in Debug.

Fix

  • Add atomic reentry guard to ensure crash handling runs only once across all entry points (SEH, SIGABRT, terminate and invalid parameter). Where subsequent entries return to prevent deadlock.

…ug due to CRT's SIGABRT handler behaviour

Scenario:
1. On Windows, the first crash is handled via UnhandledExceptionFilter -> crash_handler(EXCEPTION_POINTERS*), which wakes the reporter thread to collect and print the stack trace.
2. After printing, our handler calls abort(). In Debug CRT, abort raises SIGABRT and invokes our installed signal handler again.
3. On this second entry, crash_handler(int) sets crashed() = crash_status::crashed and waits on the condition variable. But the reporter thread has already finished (set crashed() = ending) and exited, so no one flips the state anymore. The wait blocks forever, causing a deadlock that only reproduces in Debug.

Fix
* Add atomic reentry guard to ensure crash handling runs only once across all entry points (SEH, SIGABRT, terminate and invalid parameter). Where subsequent entries return to prevent deadlock.
@bombela
Copy link
Owner

bombela commented Aug 26, 2025 via email

@ovaar
Copy link
Author

ovaar commented Aug 28, 2025

Hi @bombela

My instinct tells me backward-cpp shouldn't be responsible for this thread
synchronization. But I could be wrong. Wouldn't it be more reasonable to handle this reentrant situation in your
signal handlers? Not backward-cpp?

In my situation I use backward-cpp as my signal handlers for capturing crashes, but it is an interesting though. On the contrary, backward-cpp creates the thread, so handling situation to avoid deadlocks only seems reasonable to me.

For me it is also difficult to see the impact of the change and I was hoping you could help me with that 😄, which is why I only chose to implement the fix for c++11. On the note of c++ I personally would vouch for the absolute minimal required c++ version to be 11.

Thank you for time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants