Show debugger backtraces when C++ tests crash in Github CI (#2609)

Author: FooBarWidget

.github/workflows/main.yml

@@ -40,10 +40,10 @@ jobs:
           ARCH_AND_OS: x86_64-unknown-linux-musl
           SCCACHE_AZURE_CONNECTION_STRING: ${{ secrets.AZURE_CI_STORAGE_CONNECTION_STRING }}
 
-      - name: Setup library dependencies
+      - name: Setup dependencies
         run: >
           sudo apt update &&
-          sudo apt install -y libcurl4-openssl-dev
+          sudo apt install -y gdb libcurl4-openssl-dev
 
       - name: Setup misc
         run: |
@@ -55,17 +55,28 @@ jobs:
         run: bundle exec drake -j4 test:cxx:build
 
       - name: Run C++ tests
+        # The AbortHandler doesn't work so well on Github's Linux runners:
+        # - Triggering the original signal handler makes the runner freeze, so we force
+        #   terminate the process.
+        # - force killing the process may not work, so we enforce a low timeout to prevent
+        #   the test from haging for a long time. We enforce this primarily using the 'timeout'
+        #   command from coreutils instead of Github Actions' timeout, because the latter
+        #   could put the entire job in the "skipped" state and refuse to show logs.
+        timeout-minutes: 11
         run: >
           sudo env
           PATH="$PATH"
           GEM_PATH="$GEM_PATH"
+          PASSENGER_DUMP_CRASH_WATCH_TO_STDERR=true
+          PASSENGER_FORCE_TERMINATE_ON_ABORT=true
+          /usr/bin/timeout --signal KILL --verbose 600
           ../buildout/test/cxx/main
         working-directory: test
 
       - name: Archive logs
         uses: actions/upload-artifact@v4
         with:
-          name: cxx-test-logs-macos
+          name: cxx-test-logs-linux
           path: 'buildout/testlogs/*'
           if-no-files-found: ignore
         if: '!cancelled()'
@@ -116,18 +127,17 @@ jobs:
 
       - name: Run C++ tests
         # The AbortHandler doesn't work so well on Github's macOS runners:
-        # - crash-watch invocation freezes the runner, so we disable this.
         # - resuming the parent process after SIGSTOP doesn't work, so we force kill the parent.
         # - force killing the parent may not work, so we enforce a low timeout to prevent
         #   the test from haging for a long time. We enforce this primarily using the 'timeout'
-        #   command from coreutils instead of Github Actions' timeout,  because the latter
+        #   command from coreutils instead of Github Actions' timeout, because the latter
         #   could put the entire job in the "skipped" state and refuse to show logs.
         timeout-minutes: 11
         run: >
           sudo env
           PATH="$PATH"
           GEM_PATH="$GEM_PATH"
-          PASSENGER_DUMP_WITH_CRASH_WATCH=false
+          PASSENGER_DUMP_CRASH_WATCH_TO_STDERR=true
           PASSENGER_FORCE_TERMINATE_ON_ABORT=true
           gtimeout --signal KILL --verbose 600
           ../buildout/test/cxx/main

src/agent/Shared/Fundamentals/AbortHandler.cpp

@@ -550,10 +550,10 @@ dumpFileDescriptorInfo(AbortHandlerWorkingState &state) {
 }
 
 static void
-dumpWithCrashWatch(AbortHandlerWorkingState &state) {
+dumpWithCrashWatch(AbortHandlerWorkingState &state, bool toStderr) {
 	int fd = -1;
 
-	if (state.crashLogDirFd != -1) {
+	if (state.crashLogDirFd != -1 && !toStderr) {
 		fd = openat(state.crashLogDirFd, "backtrace.log", O_WRONLY | O_CREAT | O_TRUNC, 0600);
 		if (fd != -1) {
 			printCrashLogFileCreated(state, "backtrace.log");
@@ -594,7 +594,25 @@ dumpWithCrashWatch(AbortHandlerWorkingState &state) {
 		write_nowarn(STDERR_FILENO, state.messageBuf, pos - state.messageBuf);
 
 	} else {
-		waitpid(child, NULL, 0);
+		int status = -1;
+		int ret = waitpid(child, &status, 0);
+		int e = errno;
+		if (ret == -1 || status != 0) {
+			pos = state.messageBuf;
+			pos = ASSU::appendData(pos, end, "ERROR running 'crash-watch' (");
+			if (ret == -1) {
+				pos = ASSU::appendData(pos, end, "waitpid() failed, errno=");
+				pos = ASSU::appendInteger<int, 10>(pos, end, e);
+			} else if (WIFSIGNALED(status)) {
+				pos = ASSU::appendData(pos, end, "exited with signal ");
+				pos = ASSU::appendInteger<int, 10>(pos, end, WTERMSIG(status));
+			} else {
+				pos = ASSU::appendData(pos, end, "exit status ");
+				pos = ASSU::appendInteger<int, 10>(pos, end, WEXITSTATUS(status));
+			}
+			pos = ASSU::appendData(pos, end, ")\n");
+			write_nowarn(STDERR_FILENO, state.messageBuf, pos - state.messageBuf);
+		}
 	}
 
 	if (fd != -1) {
@@ -852,7 +870,7 @@ dumpDiagnostics(AbortHandlerWorkingState &state) {
 			pos = ASSU::appendData(pos, end, " ] Dumping a backtrace with crash-watch...\n");
 		#endif
 		write_nowarn(STDERR_FILENO, state.messageBuf, pos - state.messageBuf);
-		dumpWithCrashWatch(state);
+		dumpWithCrashWatch(state, ctx->config->dumpCrashWatchToStderr);
 	} else {
 		write_nowarn(STDERR_FILENO, "\n", 1);
 	}
@@ -969,7 +987,7 @@ forkAndRedirectToTeeAndMainLogFile(const char *crashLogDir) {
 		execlp("/usr/bin/cat", "cat", (char *) 0);
 		ASSU::printError("ERROR: cannot execute 'tee' or 'cat'; crash log will be lost!\n");
 		_exit(1);
-		return false;
+		return false; // Unreachable
 	} else if (pid == -1) {
 		ASSU::printError("ERROR: cannot fork a process for executing 'tee'\n");
 		return false;

src/agent/Shared/Fundamentals/AbortHandler.h

@@ -26,8 +26,6 @@
 #ifndef _PASSENGER_AGENT_FUNDAMENTALS_ABORT_HANDLER_H_
 #define _PASSENGER_AGENT_FUNDAMENTALS_ABORT_HANDLER_H_
 
-#include <cstddef>
-
 namespace Passenger {
 	class ResourceLocator;
 }
@@ -38,7 +36,7 @@ namespace Fundamentals {
 
 
 struct AbortHandlerConfig {
-	static const unsigned int MAX_DIAGNOSTICS_DUMPERS = 5;
+	static constexpr unsigned int MAX_DIAGNOSTICS_DUMPERS = 5;
 	typedef void (*DiagnosticsDumperFunc)(void *userData);
 
 	struct DiagnosticsDumper {
@@ -56,26 +54,16 @@ struct AbortHandlerConfig {
 	};
 
 
-	char *ruby;
-	char **origArgv;
-	unsigned int randomSeed;
-	bool dumpWithCrashWatch;
-	bool beep;
-	bool stopProcess;
-	bool forceTerminateProcess;
-	ResourceLocator *resourceLocator;
+	char *ruby = nullptr;
+	char **origArgv = nullptr;
+	unsigned int randomSeed = 0;
+	bool dumpWithCrashWatch = false;
+	bool dumpCrashWatchToStderr = false;
+	bool beep = false;
+	bool stopProcess = false;
+	bool forceTerminateProcess = false;
+	ResourceLocator *resourceLocator = nullptr;
 	DiagnosticsDumper diagnosticsDumpers[MAX_DIAGNOSTICS_DUMPERS];
-
-	AbortHandlerConfig()
-		: ruby(NULL),
-		  origArgv(NULL),
-		  randomSeed(0),
-		  dumpWithCrashWatch(false),
-		  beep(false),
-		  stopProcess(false),
-		  forceTerminateProcess(false),
-		  resourceLocator(NULL)
-		{ }
 };
 
 void installAbortHandler(const AbortHandlerConfig *config);

src/agent/Shared/Fundamentals/Initialization.cpp

@@ -465,6 +465,7 @@ maybeInitializeAbortHandler() {
 	config->origArgv = context->origArgv;
 	config->randomSeed = context->randomSeed;
 	config->dumpWithCrashWatch = getEnvBool("PASSENGER_DUMP_WITH_CRASH_WATCH", true);
+	config->dumpCrashWatchToStderr = getEnvBool("PASSENGER_DUMP_CRASH_WATCH_TO_STDERR", false);
 	config->beep = getEnvBool("PASSENGER_BEEP_ON_ABORT");
 	config->stopProcess = getEnvBool("PASSENGER_STOP_ON_ABORT");
 	config->forceTerminateProcess = getEnvBool("PASSENGER_FORCE_TERMINATE_ON_ABORT");