From cd5aa47e0547d1bc7b093fb6cdd0582eae78b391 Mon Sep 17 00:00:00 2001 From: ABeltramo Date: Fri, 15 Apr 2022 14:27:02 +0200 Subject: [PATCH] feat: added signal handlers, gracefully shutdown, stacktrace on exceptions, saving configs --- .github/workflows/linux-build-test.yml | 12 ++--- README.md | 2 +- src/wolf/CMakeLists.txt | 24 +++++++++- src/wolf/rest/servers.cpp | 22 --------- src/wolf/wolf.cpp | 64 ++++++++++++++++++++++---- 5 files changed, 84 insertions(+), 40 deletions(-) diff --git a/.github/workflows/linux-build-test.yml b/.github/workflows/linux-build-test.yml index 572de375..be9ad578 100644 --- a/.github/workflows/linux-build-test.yml +++ b/.github/workflows/linux-build-test.yml @@ -18,21 +18,21 @@ jobs: std: [ 17 ] include: - cxx: g++-9 - other_pkgs: g++-9 libboost-thread-dev libboost-filesystem-dev libboost-log-dev libssl-dev + other_pkgs: g++-9 libboost-thread-dev libboost-filesystem-dev libboost-log-dev libssl-dev libboost-stacktrace-dev - cxx: g++-10 - other_pkgs: g++-10 libboost-thread-dev libboost-filesystem-dev libboost-log-dev libssl-dev + other_pkgs: g++-10 libboost-thread-dev libboost-filesystem-dev libboost-log-dev libssl-dev libboost-stacktrace-dev - cxx: clang++-9 - other_pkgs: clang-9 libboost-thread-dev libboost-filesystem-dev libboost-log-dev libssl-dev + other_pkgs: clang-9 libboost-thread-dev libboost-filesystem-dev libboost-log-dev libssl-dev libboost-stacktrace-dev - cxx: clang++-10 - other_pkgs: clang-10 libboost-thread-dev libboost-filesystem-dev libboost-log-dev libssl-dev + other_pkgs: clang-10 libboost-thread-dev libboost-filesystem-dev libboost-log-dev libssl-dev libboost-stacktrace-dev - cxx: clang++-10 build_type: Debug std: 17 - other_pkgs: clang-10 libboost-thread-dev libboost-filesystem-dev libboost-log-dev libssl-dev + other_pkgs: clang-10 libboost-thread-dev libboost-filesystem-dev libboost-log-dev libssl-dev libboost-stacktrace-dev - cxx: clang++-10 build_type: Release std: 17 - other_pkgs: clang-10 libboost-thread-dev libboost-filesystem-dev libboost-log-dev libssl-dev + other_pkgs: clang-10 libboost-thread-dev libboost-filesystem-dev libboost-log-dev libssl-dev libboost-stacktrace-dev steps: - uses: actions/checkout@v2 diff --git a/README.md b/README.md index 385930a8..9560d7ee 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,7 @@ Unit tests will live under `tests/`, they'll be focused on testing the library p ## Dependencies ``` -cmake g++-10 gcc-10 libpipewire-0.3-dev libboost-thread-dev libboost-filesystem-dev libboost-log-dev libssl-dev +cmake g++-10 gcc-10 libpipewire-0.3-dev libboost-thread-dev libboost-filesystem-dev libboost-log-dev libssl-dev libboost-stacktrace-dev ``` ## Acknowledgements diff --git a/src/wolf/CMakeLists.txt b/src/wolf/CMakeLists.txt index 9a2c6787..467f74f7 100644 --- a/src/wolf/CMakeLists.txt +++ b/src/wolf/CMakeLists.txt @@ -12,9 +12,31 @@ FetchContent_Declare( FetchContent_MakeAvailable(fmtlib) -find_package(Boost REQUIRED COMPONENTS log_setup log) +find_package(Boost + REQUIRED COMPONENTS + log_setup + log + stacktrace_basic) include_directories(${Boost_INCLUDE_DIRS}) + +### Boost stacktrace exception handling +# adapted from https://github.com/jschueller/boost-stacktrace-example/blob/master/CMakeLists.txt +# also see: https://www.boost.org/doc/libs/develop/doc/html/stacktrace/configuration_and_build.html +find_library(BOOST_STACKTRACE_BACKTRACE_LIBRARY NAMES boost_stacktrace_backtrace) +if (UNIX) + message(STATUS "Using Boost::stacktrace/addr2line") + add_definitions(-D_GNU_SOURCE=1) + target_compile_definitions(wolf PRIVATE BOOST_STACKTRACE_USE_ADDR2LINE) + target_link_libraries(wolf PRIVATE dl) +elseif (MINGW AND BOOST_STACKTRACE_BACKTRACE_LIBRARY) + message(STATUS "Using Boost::stacktrace/backtrace") + target_compile_definitions(wolf PRIVATE BOOST_STACKTRACE_USE_BACKTRACE) + target_link_libraries(wolf PRIVATE boost_stacktrace_backtrace backtrace) +else () + message(STATUS "Using Boost::stacktrace/basic") +endif () + # Some libraries don't work with FetchContent_Declare out of the box # We have to manually git clone them and adding them into `libraries` target_include_directories(wolf PRIVATE libraries .) diff --git a/src/wolf/rest/servers.cpp b/src/wolf/rest/servers.cpp index 6eaedff3..e5874030 100644 --- a/src/wolf/rest/servers.cpp +++ b/src/wolf/rest/servers.cpp @@ -14,28 +14,6 @@ using HttpServer = SimpleWeb::Server; namespace HTTPServers { -/** - * @brief Create an HTTPS server - * - * @param pkey_filename - * @param cert_filename - * @return std::unique_ptr - */ -std::unique_ptr createHTTPS(const std::string &pkey_filename, - const std::string &cert_filename, - const std::shared_ptr &config) { - return std::make_unique(cert_filename, pkey_filename, config); -} - -/** - * @brief Create an HTTP server - * - * @return std::unique_ptr - */ -std::unique_ptr createHTTP() { - return std::make_unique(); -} - /** * @brief Start the generic server on the specified port * @return std::thread: the thread where this server will run diff --git a/src/wolf/wolf.cpp b/src/wolf/wolf.cpp index 03a4acf0..c7c6d1c2 100644 --- a/src/wolf/wolf.cpp +++ b/src/wolf/wolf.cpp @@ -1,9 +1,11 @@ -#include - +#include +#include +#include #include #include #include #include +#include /** * @brief Will try to load the config file and fallback to defaults @@ -58,17 +60,45 @@ initialize(const std::string &config_file, const std::string &pkey_filename, con return std::make_shared(state); } +/** + * Taken from: https://stackoverflow.com/questions/11468414/using-auto-and-lambda-to-handle-signal + * in order to have pass a lambda with variable capturing + */ +namespace { +std::function shutdown_handler; +void signal_handler(int signal) { + shutdown_handler(signal); +} +} // namespace + +/** + * @brief: if an exception was raised we should have created a dump file, here we can pretty print it + */ +void check_exceptions() { + if (boost::filesystem::exists("./backtrace.dump")) { + std::ifstream ifs("./backtrace.dump"); + + auto st = boost::stacktrace::stacktrace::from_dump(ifs); + logs::log(logs::error, "Previous run crashed: \n{}", to_string(st)); + + // cleaning up + ifs.close(); + boost::filesystem::remove("./backtrace.dump"); + } +} + /** * @brief here's where the magic starts */ int main(int argc, char *argv[]) { logs::init(logs::trace); + check_exceptions(); auto config_file = "config.json"; auto local_state = initialize(config_file, "key.pem", "cert.pem"); - auto https_server = HTTPServers::createHTTPS("key.pem", "cert.pem", local_state->config); - auto http_server = HTTPServers::createHTTP(); + auto https_server = std::make_unique("cert.pem", "key.pem", local_state->config); + auto http_server = std::make_unique(); auto https_thread = HTTPServers::startServer(https_server.get(), *local_state, @@ -77,12 +107,26 @@ int main(int argc, char *argv[]) { *local_state, local_state->config->map_port(moonlight::Config::HTTP_PORT)); - https_thread.join(); - http_thread.join(); + // Exception and termination handling + shutdown_handler = [&](int signum) { + logs::log(logs::info, "Received interrupt signal {}, clean exit", signum); + if (signum == SIGABRT || signum == SIGSEGV) { + auto trace_file = "./backtrace.dump"; + logs::log(logs::info, "Dumping stacktrace to {}", trace_file); + boost::stacktrace::safe_dump_to(trace_file); + } + + logs::log(logs::info, "Saving back current configuration to file: {}", config_file); + local_state->config->saveCurrentConfig(config_file); - // TODO: clean exit + exit(signum); + }; + std::signal(SIGINT, signal_handler); + std::signal(SIGTERM, signal_handler); + std::signal(SIGQUIT, signal_handler); + std::signal(SIGSEGV, signal_handler); + std::signal(SIGABRT, signal_handler); - logs::log(logs::info, "Saving back current configuration to file: {}", config_file); - local_state->config->saveCurrentConfig(config_file); - return 0; + https_thread.join(); + http_thread.join(); } \ No newline at end of file