Skip to content

Commit

Permalink
fix: only use async-signal-safe functions in the shutdown_handler
Browse files Browse the repository at this point in the history
  • Loading branch information
ABeltramo committed Jan 22, 2024
1 parent d2ff748 commit c372e73
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 38 deletions.
40 changes: 24 additions & 16 deletions src/moonlight-server/exceptions/exceptions.h
Original file line number Diff line number Diff line change
@@ -1,39 +1,47 @@
/**
* Implements safe stacktrace dumping and loading.
*
* Only uses async-signal-safe functions for dumping the stacktrace; read here for more info:
* https://man7.org/linux/man-pages/man7/signal-safety.7.html
*/
#pragma once

#include <cpptrace/cpptrace.hpp>
#include <fcntl.h>
#include <helpers/logger.hpp>
#include <istream>
#include <ostream>
#include <unistd.h>

static void safe_dump_stacktrace_to(std::ostream &out) {
static void safe_dump_stacktrace_to(const std::string &file_name) {
constexpr std::size_t N = 100;
cpptrace::frame_ptr buffer[N];
std::size_t count = cpptrace::safe_generate_raw_trace(buffer, N);
if (count > 0) {
out << count << '\n';
int fd = open(file_name.c_str(), O_WRONLY | O_CREAT | O_DSYNC);
if (fd <= 0) {
return;
}
write(fd, reinterpret_cast<char *>(&count), sizeof(count));
for (std::size_t i = 0; i < count; i++) {
cpptrace::safe_object_frame frame{};
cpptrace::get_safe_object_frame(buffer[i], &frame);
out << frame.address_relative_to_object_start << ' ' << frame.raw_address << ' ';
out.write(frame.object_path, sizeof(frame.object_path));
out << '\n';
write(fd, &frame, sizeof(frame));
}
close(fd);
}
}

static std::unique_ptr<cpptrace::object_trace> load_stacktrace_from(std::istream &in) {
static std::unique_ptr<cpptrace::object_trace> load_stacktrace_from(const std::string &file_name) {
cpptrace::object_trace trace{};
std::size_t count;
in >> count;
in.ignore(); // ignore newline
int fd = open(file_name.c_str(), O_RDONLY | O_CREAT | O_DSYNC);
if (fd <= 0) {
logs::log(logs::warning, "Unable to open stacktrace file {}", file_name);
return {};
}
read(fd, reinterpret_cast<char *>(&count), sizeof(count));
for (std::size_t i = 0; i < count; i++) {
cpptrace::safe_object_frame frame{};
in >> frame.address_relative_to_object_start;
in.ignore(); // ignore space
in >> frame.raw_address;
in.ignore(); // ignore space
in.read(frame.object_path, sizeof(frame.object_path));
in.ignore(); // ignore newline
read(fd, &frame, sizeof(frame));
try {
trace.frames.push_back(frame.resolve());
} catch (std::exception &ex) {
Expand Down
18 changes: 6 additions & 12 deletions src/moonlight-server/wolf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <csignal>
#include <exceptions/exceptions.h>
#include <filesystem>
#include <fstream>
#include <gst-plugin/video.hpp>
#include <immer/array.hpp>
#include <immer/array_transient.hpp>
Expand Down Expand Up @@ -403,20 +402,17 @@ auto setup_sessions_handlers(const immer::box<state::AppState> &app_state,
}

static std::string backtrace_file_src() {
return fmt::format("{}/backtrace.dump", utils::get_env("WOLF_CFG_FOLDER", "."));
return std::string(utils::get_env("WOLF_CFG_FOLDER", ".")) + "/backtrace.dump"s;
}

/**
* Keep this as small as possible, make sure to only use async-signal-safe functions
*/
static void shutdown_handler(int signum) {
logs::log(logs::info, "Received interrupt signal {}, clean exit", signum);
if (signum == SIGABRT || signum == SIGSEGV) {
auto stack_file = backtrace_file_src();
logs::log(logs::error, "Runtime error, dumping stacktrace to {}", stack_file);
std::ofstream fs(stack_file);
safe_dump_stacktrace_to(fs);
fs.close();
safe_dump_stacktrace_to(stack_file);
}

logs::log(logs::info, "See ya!");
exit(signum);
}

Expand All @@ -426,9 +422,7 @@ static void shutdown_handler(int signum) {
static void check_exceptions() {
auto stack_file = backtrace_file_src();
if (boost::filesystem::exists(stack_file)) {
std::ifstream ifs(stack_file);
load_stacktrace_from(ifs)->resolve().print();
ifs.close();
load_stacktrace_from(stack_file)->resolve().print();
auto now = std::chrono::system_clock::now();
boost::filesystem::rename(
stack_file,
Expand Down
19 changes: 9 additions & 10 deletions tests/testExceptions.cpp
Original file line number Diff line number Diff line change
@@ -1,21 +1,20 @@
#include "catch2/catch_all.hpp"
using Catch::Matchers::EndsWith;
using Catch::Matchers::Contains;
using Catch::Matchers::Equals;

#include <exceptions/exceptions.h>
#include <fstream>
#include <iostream>

TEST_CASE("Exceptions", "[Exceptions]") {
std::ofstream ofs("stacktrace.txt");
safe_dump_stacktrace_to(ofs);
ofs.close();
safe_dump_stacktrace_to("stacktrace.txt");

std::ifstream ifs("stacktrace.txt");
auto trace = load_stacktrace_from(ifs);
REQUIRE(trace->frames.size() > 0);
auto trace = load_stacktrace_from("stacktrace.txt");

auto stacktrace = trace->resolve();
REQUIRE(stacktrace.frames.size() > 0);
REQUIRE_THAT(stacktrace.frames[0].filename, EndsWith("src/moonlight-server/exceptions/exceptions.h"));
REQUIRE_THAT(stacktrace.frames[0].symbol, Equals("safe_dump_stacktrace_to"));
stacktrace.print(std::cout, false);

// TODO: seems to be different when running on Github Actions
// REQUIRE_THAT(stacktrace.frames[0].filename, EndsWith("src/moonlight-server/exceptions/exceptions.h"));
// REQUIRE_THAT(stacktrace.frames[0].symbol, Equals("safe_dump_stacktrace_to"));
}

0 comments on commit c372e73

Please sign in to comment.