diff options
| author | virchau13 <virchau13@hexular.net> | 2024-04-28 00:38:48 +0800 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-04-27 17:38:48 +0100 |
| commit | 90a53aed59be6bac45c13eac954471d56ac23375 (patch) | |
| tree | c1a246f6cdf0b9f6d0f1f6ea6f282b4fa5666976 /src/debug/CrashReporter.cpp | |
| parent | 55490637aaf13c8400a0679fcf5b7fca417bc923 (diff) | |
CrashReporter: fix deadlocks by making it mostly async-signal-safe (#5771)
`CrashReporter::createAndSaveCrash()` is not async-signal-safe,
resulting in random deadlocks/double-crashes during Hyprland crashes.
This changes the function to be (mostly) async-signal-safe.
Diffstat (limited to 'src/debug/CrashReporter.cpp')
| -rw-r--r-- | src/debug/CrashReporter.cpp | 209 |
1 files changed, 134 insertions, 75 deletions
diff --git a/src/debug/CrashReporter.cpp b/src/debug/CrashReporter.cpp index a4bdc761..ce1a92ba 100644 --- a/src/debug/CrashReporter.cpp +++ b/src/debug/CrashReporter.cpp @@ -1,59 +1,127 @@ #include "CrashReporter.hpp" -#include <random> +#include <fcntl.h> #include <sys/utsname.h> -#include <fstream> -#include <signal.h> #include <link.h> +#include <time.h> +#include <errno.h> +#include <sys/stat.h> #include "../plugins/PluginSystem.hpp" +#include "../signal-safe.hpp" #if defined(__DragonFly__) || defined(__FreeBSD__) || defined(__NetBSD__) #include <sys/sysctl.h> #endif -std::string getRandomMessage() { - - const std::vector<std::string> MESSAGES = {"Sorry, didn't mean to...", - "This was an accident, I swear!", - "Calm down, it was a misinput! MISINPUT!", - "Oops", - "Vaxry is going to be upset.", - "Who tried dividing by zero?!", - "Maybe you should try dusting your PC in the meantime?", - "I tried so hard, and got so far...", - "I don't feel so good...", - "*thud*", - "Well this is awkward.", - "\"stable\"", - "I hope you didn't have any unsaved progress.", - "All these computers..."}; - - std::random_device dev; - std::mt19937 engine(dev()); - std::uniform_int_distribution<> distribution(0, MESSAGES.size() - 1); - - return MESSAGES[distribution(engine)]; +static char const* const MESSAGES[] = {"Sorry, didn't mean to...", + "This was an accident, I swear!", + "Calm down, it was a misinput! MISINPUT!", + "Oops", + "Vaxry is going to be upset.", + "Who tried dividing by zero?!", + "Maybe you should try dusting your PC in the meantime?", + "I tried so hard, and got so far...", + "I don't feel so good...", + "*thud*", + "Well this is awkward.", + "\"stable\"", + "I hope you didn't have any unsaved progress.", + "All these computers..."}; + +// <random> is not async-signal-safe, fake it with time(NULL) instead +char const* getRandomMessage() { + return MESSAGES[time(NULL) % (sizeof(MESSAGES) / sizeof(MESSAGES[0]))]; +} + +[[noreturn]] inline void exit_with_error(char const* err) { + write(STDERR_FILENO, err, strlen(err)); + // perror() is not signal-safe, but we use it here + // because if the crash-handler already crashed, it can't get any worse. + perror(""); + abort(); } void CrashReporter::createAndSaveCrash(int sig) { + int reportFd; + + // We're in the signal handler, so we *only* have stack memory. + // To save as much stack memory as possible, + // destroy things as soon as possible. + { + MaxLengthCString<255> reportPath; + + const auto HOME = sig_getenv("HOME"); + const auto CACHE_HOME = sig_getenv("XDG_CACHE_HOME"); + + if (CACHE_HOME && CACHE_HOME[0] != '\0') { + reportPath += CACHE_HOME; + reportPath += "/hyprland"; + } else if (HOME && HOME[0] != '\0') { + reportPath += HOME; + reportPath += "/.cache/hyprland"; + } else { + exit_with_error("$CACHE_HOME and $HOME not set, nowhere to report crash\n"); + return; + } - // get the backtrace - const int PID = getpid(); + int ret = mkdir(reportPath.get_str(), S_IRWXU); + //__asm__("int $3"); + if (ret < 0 && errno != EEXIST) { + exit_with_error("failed to mkdir() crash report directory\n"); + } + reportPath += "/hyprlandCrashReport"; + reportPath.write_num(getpid()); + reportPath += ".txt"; + + { + BufFileWriter<64> stderr(2); + stderr += "Hyprland has crashed :( Consult the crash report at "; + if (!reportPath.boundsExceeded()) { + stderr += reportPath.get_str(); + } else { + stderr += "[ERROR: Crash report path does not fit into memory! Check if your $CACHE_HOME/$HOME is too deeply nested. Max 255 characters.]"; + } + stderr += " for more information.\n"; + stderr.flush(); + } - std::string finalCrashReport = ""; + reportFd = open(reportPath.get_str(), O_WRONLY | O_CREAT, S_IRWXU); + if (reportFd < 0) { + exit_with_error("Failed to open crash report path for writing"); + } + } + BufFileWriter<512> finalCrashReport(reportFd); finalCrashReport += "--------------------------------------------\n Hyprland Crash Report\n--------------------------------------------\n"; - finalCrashReport += getRandomMessage() + "\n\n"; - - finalCrashReport += std::format("Hyprland received signal {} ({})\n\n", sig, (const char*)strsignal(sig)); - - finalCrashReport += std::format("Version: {}\nTag: {}\n\n", GIT_COMMIT_HASH, GIT_TAG); - - if (g_pPluginSystem && !g_pPluginSystem->getAllPlugins().empty()) { + finalCrashReport += getRandomMessage(); + finalCrashReport += "\n\n"; + + finalCrashReport += "Hyprland received signal "; + finalCrashReport.writeNum(sig); + finalCrashReport += '('; + finalCrashReport += sig_strsignal(sig); + finalCrashReport += ")\nVersion: "; + finalCrashReport += GIT_COMMIT_HASH; + finalCrashReport += "\nTag: "; + finalCrashReport += GIT_TAG; + finalCrashReport += "\n\n"; + + if (g_pPluginSystem && g_pPluginSystem->pluginCount() > 0) { finalCrashReport += "Hyprland seems to be running with plugins. This crash might not be Hyprland's fault.\nPlugins:\n"; - for (auto& p : g_pPluginSystem->getAllPlugins()) { - finalCrashReport += std::format("\t{} ({}) {}\n", p->name, p->author, p->version); + size_t count = g_pPluginSystem->pluginCount(); + CPlugin* plugins[count]; + g_pPluginSystem->sig_getPlugins(plugins, count); + + for (size_t i = 0; i < count; i++) { + auto p = plugins[i]; + finalCrashReport += '\t'; + finalCrashReport += p->name; + finalCrashReport += " ("; + finalCrashReport += p->author; + finalCrashReport += ") "; + finalCrashReport += p->version; + finalCrashReport += '\n'; } finalCrashReport += "\n\n"; @@ -61,21 +129,36 @@ void CrashReporter::createAndSaveCrash(int sig) { finalCrashReport += "System info:\n"; - struct utsname unameInfo; - uname(&unameInfo); - - finalCrashReport += std::format("\tSystem name: {}\n\tNode name: {}\n\tRelease: {}\n\tVersion: {}\n\n", std::string{unameInfo.sysname}, std::string{unameInfo.nodename}, - std::string{unameInfo.release}, std::string{unameInfo.version}); + { + struct utsname unameInfo; + uname(&unameInfo); + + finalCrashReport += "\tSystem name: "; + finalCrashReport += unameInfo.sysname; + finalCrashReport += "\n\tNode name: "; + finalCrashReport += unameInfo.nodename; + finalCrashReport += "\n\tRelease: "; + finalCrashReport += unameInfo.release; + finalCrashReport += "\n\tVersion: "; + finalCrashReport += unameInfo.version; + finalCrashReport += "\n\n"; + } + finalCrashReport += "GPU:\n\t"; #if defined(__DragonFly__) || defined(__FreeBSD__) - const std::string GPUINFO = execAndGet("pciconf -lv | fgrep -A4 vga"); + finalCrashReport.writeCmdOutput("pciconf -lv | fgrep -A4 vga"); #else - const std::string GPUINFO = execAndGet("lspci -vnn | grep VGA"); + finalCrashReport.writeCmdOutput("lspci -vnn | grep VGA"); #endif - finalCrashReport += "GPU:\n\t" + GPUINFO; + finalCrashReport += "\n\nos-release:\n"; + finalCrashReport.writeCmdOutput("cat /etc/os-release | sed 's/^/\t/'"); - finalCrashReport += std::format("\n\nos-release:\n\t{}\n\n\n", replaceInString(execAndGet("cat /etc/os-release"), "\n", "\n\t")); + // dladdr1()/backtrace_symbols()/this entire section allocates, and hence is NOT async-signal-safe. + // Make sure that we save the current known crash report information, + // so that if we are caught in a deadlock during a call to malloc(), + // there is still something to debug from. + finalCrashReport.flush(); finalCrashReport += "Backtrace:\n"; @@ -132,7 +215,10 @@ void CrashReporter::createAndSaveCrash(int sig) { std::stringstream ssin(ADDR2LINE); for (size_t i = 0; i < CALLSTACK.size(); ++i) { - finalCrashReport += std::format("\t#{} | {}", i, CALLSTACK[i].desc); + finalCrashReport += "\t#"; + finalCrashReport.writeNum(i); + finalCrashReport += " | "; + finalCrashReport += CALLSTACK[i].desc; std::string functionInfo; std::string fileLineInfo; std::getline(ssin, functionInfo); @@ -142,32 +228,5 @@ void CrashReporter::createAndSaveCrash(int sig) { finalCrashReport += "\n\nLog tail:\n"; - finalCrashReport += Debug::rollingLog.substr(Debug::rollingLog.find("\n") + 1); - - const auto HOME = getenv("HOME"); - const auto CACHE_HOME = getenv("XDG_CACHE_HOME"); - - if (!HOME) - return; - - std::ofstream ofs; - std::string reportDir; - - if (!CACHE_HOME || std::string(CACHE_HOME).empty()) - reportDir = std::string(HOME) + "/.cache/hyprland"; - else - reportDir = std::string(CACHE_HOME) + "/hyprland"; - - if (!std::filesystem::exists(reportDir)) - std::filesystem::create_directory(reportDir); - const auto path = reportDir + "/hyprlandCrashReport" + std::to_string(PID) + ".txt"; - - ofs.open(path, std::ios::trunc); - - ofs << finalCrashReport; - - ofs.close(); - - Debug::disableStdout = false; - Debug::log(CRIT, "Hyprland has crashed :( Consult the crash report at {} for more information.", path); + finalCrashReport += std::string_view(Debug::rollingLog).substr(Debug::rollingLog.find("\n") + 1); } |
