Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backtraces #3456

Merged
merged 3 commits into from
May 10, 2024
Merged

Backtraces #3456

merged 3 commits into from
May 10, 2024

Conversation

benjaminwinger
Copy link
Collaborator

@benjaminwinger benjaminwinger commented May 7, 2024

Fixes #3053 (currently missing backtrace on segfaults). Everything is working except backtraces for segfaults on windows.

Adds backtracing using cpptrace, and makes the default configuration for tests RelWithDebInfo(otherwise the backtraces aren't very useful).
Backtraces are being printed in the kuzu::common::Exception constructor since including them in the message will break the exception message tests. Unfortunately this does mean that the stack trace is printed before the exception, but is printed backwards with the expectation that it will be printed after the exception. It also means that exceptions which are handled and not re-thrown will also have their stack traces printed.

Safe signal handling using a separate executable as described here was working, except that when it couldn't find the executable it would hang (in a suspiciously similar way to #3459, where only the scheduling thread can be interrupted in gdb). I thought that it might be more reliable just to use the unsafe cpptrace::generate_trace().print(), and deal with the possibility of a CI job hanging on rare occasions (mostly it seems fine, but as far as I understand, major memory issues could cause various problems including hangs if we try and do complex stuff like memory allocation when handling the signal).

@benjaminwinger benjaminwinger force-pushed the backtraces branch 9 times, most recently from 2c48dc1 to e08ccb3 Compare May 9, 2024 18:17
@benjaminwinger benjaminwinger marked this pull request as ready for review May 9, 2024 18:40
Currently does so by printing them in the exception constructor, as otherwise we wouldn't be able to check for expected query result messages.
Also adds a std::terminate handler to handle some unexpected exits which would otherwise not print traces
Copy link

codecov bot commented May 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.05%. Comparing base (b491af2) to head (ce77b59).
Report is 64 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3456      +/-   ##
==========================================
- Coverage   92.59%   90.05%   -2.54%     
==========================================
  Files        1205     1160      -45     
  Lines       44825    42357    -2468     
==========================================
- Hits        41504    38144    -3360     
- Misses       3321     4213     +892     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@ray6080 ray6080 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one!

@benjaminwinger benjaminwinger merged commit 4e1a9fd into master May 10, 2024
20 checks passed
@benjaminwinger benjaminwinger deleted the backtraces branch May 10, 2024 19:23
ted-wq-x pushed a commit to ted-wq-x/kuzu that referenced this pull request Nov 14, 2024
* Print backtraces when exceptions are thrown

Currently does so by printing them in the exception constructor, as otherwise we wouldn't be able to check for expected query result messages.
Also adds a std::terminate handler to handle some unexpected exits which would otherwise not print traces

* Add support for SIGSEGV backtraces (and other signals)

* Ignore false positive GCC warning in relwithdebinfo mode

(cherry picked from commit 4e1a9fd)
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.

[PROPOSAL] Backtrace
2 participants