Fix errors with handling SIGSEGV and SIGABRT
Summary: There were a couple of issues with handling the above 2 signals. 1) Calling `std::exit` from a signal handler is undefined behaviour. The only defined way for a signal handler to stop the program is calling one of: `std::_Exit`, `std::abort` or `std::quick_exit`. Neither of them will completely clean the resources, so a clean exit is not possible. Since SIGSEGV and SIGABRT happen in extraordinary circumstances that we wish to debug 99% of the time, it makes sense to generate a core dump which can be inspected by a debugger. Of the 3 termination functions, only `std::abort` will generate a core dump, so it makes sense to use that to stop the program. Also, since we are now aborting as is the default behaviour on SIGSEGV and SIGABRT, it becomes questionable why have a custom handler at all. 2) Raising an exception inside a signal handler is undefined behaviour Although the handler by itself does not raise an exception, it is possible for the logging facility to raise one. This is a real case when logging a stack trace in particular. Stack trace is generated by creating a string "<function name> <line location>". It is possible that a function name will contain '{}' somewhere inside. This is usually the case with anonymous functions. The generated string is then passed to logging, which uses the `fmt` library to fill '{}' with remaining arguments. Since only a single argument (the stack trace string) is passed for formatting, naturally the `fmt::format` throws an exception, that it is missing a format argument. We could provide an overload which takes a single string, but that defeats the purpose of `fmt::format` raising an exception in regular code if we forget to pass an argument. Another solution is to escape the whole stack trace string, so it is valid for formatting, but this only complicates the handler even further. The simplest solution is to send the stack trace to `stderr` and avoid logging altogether. Simplify Shutdown, so it can be used in a signal handler Reviewers: florijan, mferencevic, buda, mislav.bradac Reviewed By: mferencevic, buda Subscribers: pullbot Differential Revision: https://phabricator.memgraph.io/D474
This commit is contained in:
parent
13ae13bf43
commit
527d69d382
@ -69,13 +69,15 @@ class Server
|
|||||||
while (alive_) {
|
while (alive_) {
|
||||||
this->WaitAndProcessEvents();
|
this->WaitAndProcessEvents();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
logger_.info("Shutting down...");
|
||||||
|
for (auto &worker : workers_) worker->thread_.join();
|
||||||
}
|
}
|
||||||
|
|
||||||
void Shutdown() {
|
void Shutdown() {
|
||||||
logger_.info("Shutting down...");
|
// This should be as simple as possible, so that it can be called inside a
|
||||||
|
// signal handler.
|
||||||
alive_.store(false);
|
alive_.store(false);
|
||||||
|
|
||||||
for (auto &worker : workers_) worker->thread_.join();
|
|
||||||
}
|
}
|
||||||
|
|
||||||
void OnConnect() {
|
void OnConnect() {
|
||||||
|
@ -80,14 +80,6 @@ void load_config(int &argc, char **&argv) {
|
|||||||
gflags::ParseCommandLineFlags(&argc, &argv, true);
|
gflags::ParseCommandLineFlags(&argc, &argv, true);
|
||||||
}
|
}
|
||||||
|
|
||||||
void log_stacktrace(const std::string &title) {
|
|
||||||
Stacktrace stacktrace;
|
|
||||||
// TODO: Change this from 'info' to 'error', when default error logging is
|
|
||||||
// added.
|
|
||||||
logging::info(title);
|
|
||||||
logging::info(stacktrace.dump());
|
|
||||||
}
|
|
||||||
|
|
||||||
int main(int argc, char **argv) {
|
int main(int argc, char **argv) {
|
||||||
fs::current_path(fs::path(argv[0]).parent_path());
|
fs::current_path(fs::path(argv[0]).parent_path());
|
||||||
load_config(argc, argv);
|
load_config(argc, argv);
|
||||||
@ -109,12 +101,16 @@ int main(int argc, char **argv) {
|
|||||||
|
|
||||||
// Signal handling init.
|
// Signal handling init.
|
||||||
SignalHandler::register_handler(Signal::SegmentationFault, []() {
|
SignalHandler::register_handler(Signal::SegmentationFault, []() {
|
||||||
log_stacktrace("SegmentationFault signal raised");
|
// Log that we got SIGSEGV and abort the program, because returning from
|
||||||
std::exit(EXIT_FAILURE);
|
// SIGSEGV handler is undefined behaviour.
|
||||||
|
std::cerr << "SegmentationFault signal raised" << std::endl;
|
||||||
|
std::abort(); // This will continue into our SIGABRT handler.
|
||||||
});
|
});
|
||||||
SignalHandler::register_handler(Signal::Abort, []() {
|
SignalHandler::register_handler(Signal::Abort, []() {
|
||||||
log_stacktrace("Abort signal raised");
|
// Log the stacktrace and let the abort continue.
|
||||||
std::exit(EXIT_FAILURE);
|
Stacktrace stacktrace;
|
||||||
|
std::cerr << "Abort signal raised" << std::endl
|
||||||
|
<< stacktrace.dump() << std::endl;
|
||||||
});
|
});
|
||||||
|
|
||||||
// Initialize endpoint.
|
// Initialize endpoint.
|
||||||
|
Loading…
Reference in New Issue
Block a user