CrashReporter: fix deadlocks by making it mostly async-signal-safe (h…

…yprwm#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.

src/Compositor.cpp

@@ -36,6 +36,14 @@ void handleUnrecoverableSignal(int sig) {
         return;
     }
 
+    // Kill the program if the crash-reporter is caught in a deadlock.
+    signal(SIGALRM, [](int _) {
+        char const* msg = "\nCrashReporter exceeded timeout, forcefully exiting\n";
+        write(2, msg, strlen(msg));
+        abort();
+    });
+    alarm(15);
+
     CrashReporter::createAndSaveCrash(sig);
 
     abort();

src/debug/CrashReporter.cpp

@@ -1,81 +1,164 @@
 #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";
     }
 
     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);
 }

src/plugins/PluginSystem.cpp

@@ -194,3 +194,13 @@ std::vector<CPlugin*> CPluginSystem::getAllPlugins() {
         results[i] = m_vLoadedPlugins[i].get();
     return results;
 }
+
+size_t CPluginSystem::pluginCount() {
+    return m_vLoadedPlugins.size();
+}
+
+void CPluginSystem::sig_getPlugins(CPlugin** data, size_t len) {
+    for (size_t i = 0; i < std::min(m_vLoadedPlugins.size(), len); i++) {
+        data[i] = m_vLoadedPlugins[i].get();
+    }
+}

src/plugins/PluginSystem.hpp

@@ -37,6 +37,8 @@ class CPluginSystem {
     CPlugin*                 getPluginByPath(const std::string& path);
     CPlugin*                 getPluginByHandle(HANDLE handle);
     std::vector<CPlugin*>    getAllPlugins();
+    size_t                   pluginCount();
+    void                     sig_getPlugins(CPlugin** data, size_t len);
 
     bool                     m_bAllowConfigVars = false;
     std::string              m_szLastError      = "";

src/signal-safe.cpp

@@ -0,0 +1,30 @@
+#include "signal-safe.hpp"
+
+#ifndef __GLIBC__
+#include <signal.h>
+#endif
+#include <fcntl.h>
+#include <unistd.h>
+#include <string.h>
+
+extern char** environ;
+
+char const*   sig_getenv(char const* name) {
+    int len = strlen(name);
+    for (char** var = environ; *var != NULL; var++) {
+        if (strncmp(*var, name, len) == 0 && (*var)[len] == '=') {
+            return (*var) + len + 1;
+        }
+    }
+    return NULL;
+}
+
+char const* sig_strsignal(int sig) {
+#ifdef __GLIBC__
+    return sigabbrev_np(sig);
+#elif defined(__DragonFly__) || defined(__FreeBSD__)
+    return sys_signame[sig];
+#else
+    return "unknown";
+#endif
+}