Change: recover when possible from crashes during a crash (#11238)

src/crashlog.cpp

@@ -367,11 +367,12 @@ bool CrashLog::WriteCrashLog()
  * Write the (crash) dump to a file.
  *
  * @note Sets \c crashdump_filename when there is a successful return.
- * @return 1 iff the crashdump was successfully created, -1 if it failed, 0 if not implemented.
+ * @return True iff the crashdump was successfully created.
  */
-/* virtual */ int CrashLog::WriteCrashDump()
+/* virtual */ bool CrashLog::WriteCrashDump()
 {
-	return 0;
+	fmt::print("No method to create a crash.dmp available.\n");
+	return false;
 }
 
 /**
@@ -443,7 +444,7 @@ bool CrashLog::MakeCrashLog()
 	fmt::print("Crash log generated.\n\n");
 
 	fmt::print("Writing crash log to disk...\n");
-	bool bret = this->WriteCrashLog();
+	bool bret = this->TryExecute("crashlog", [this]() { return this->WriteCrashLog(); });
 	if (bret) {
 		fmt::print("Crash log written to {}. Please add this file to any bug reports.\n\n", this->crashlog_filename);
 	} else {
@@ -452,18 +453,15 @@ bool CrashLog::MakeCrashLog()
 	}
 
 	fmt::print("Writing crash dump to disk...\n");
-	int dret = this->WriteCrashDump();
-	if (dret < 0) {
-		fmt::print("Writing crash dump failed.\n\n");
-		ret = false;
-	} else if (dret > 0) {
+	bret = this->TryExecute("crashdump", [this]() { return this->WriteCrashDump(); });
+	if (bret) {
 		fmt::print("Crash dump written to {}. Please add this file to any bug reports.\n\n", this->crashdump_filename);
 	} else {
-		fmt::print("Skipped; missing dependency to create crash dump.\n");
+		fmt::print("Writing crash dump failed.\n\n");
 	}
 
 	fmt::print("Writing crash savegame...\n");
-	bret = this->WriteSavegame();
+	bret = this->TryExecute("savegame", [this]() { return this->WriteSavegame(); });
 	if (bret) {
 		fmt::print("Crash savegame written to {}. Please add this file and the last (auto)save to any bug reports.\n\n", this->savegame_filename);
 	} else {
@@ -472,7 +470,7 @@ bool CrashLog::MakeCrashLog()
 	}
 
 	fmt::print("Writing crash screenshot...\n");
-	bret = this->WriteScreenshot();
+	bret = this->TryExecute("screenshot", [this]() { return this->WriteScreenshot(); });
 	if (bret) {
 		fmt::print("Crash screenshot written to {}. Please add this file to any bug reports.\n\n", this->screenshot_filename);
 	} else {
@@ -480,7 +478,7 @@ bool CrashLog::MakeCrashLog()
 		fmt::print("Writing crash screenshot failed.\n\n");
 	}
 
-	this->SendSurvey();
+	this->TryExecute("survey", [this]() { this->SendSurvey(); return true; });
 
 	return ret;
 }

src/crashlog.h

@@ -52,6 +52,17 @@ protected:
 
 	std::string CreateFileName(const char *ext, bool with_dir = true) const;
 
+	/**
+	 * Execute the func() and return its value. If any exception / signal / crash happens,
+	 * catch it and return false. This function should, in theory, never not return, even
+	 * in the worst conditions.
+	 *
+	 * @param section_name The name of the section to be executed. Printed when a crash happens.
+	 * @param func The function to call.
+	 * @return true iff the function returned true.
+	 */
+	virtual bool TryExecute(std::string_view section_name, std::function<bool()> &&func) = 0;
+
 public:
 	/** Stub destructor to silence some compilers. */
 	virtual ~CrashLog() = default;
@@ -65,7 +76,7 @@ public:
 	void FillCrashLog(std::back_insert_iterator<std::string> &output_iterator) const;
 	bool WriteCrashLog();
 
-	virtual int WriteCrashDump();
+	virtual bool WriteCrashDump();
 	bool WriteSavegame();
 	bool WriteScreenshot();
 

src/os/macosx/crashlog_osx.cpp

@@ -16,6 +16,7 @@
 #include "../../video/video_driver.hpp"
 #include "macos.h"
 
+#include <setjmp.h>
 #include <signal.h>
 #include <mach-o/arch.h>
 #include <dlfcn.h>
@@ -37,6 +38,9 @@
 
 #define MAX_STACK_FRAMES 64
 
+/** The signals we want our crash handler to handle. */
+static constexpr int _signals_to_handle[] = { SIGSEGV, SIGABRT, SIGFPE, SIGBUS, SIGILL, SIGSYS, SIGQUIT };
+
 /**
  * OSX implementation for the crash logger.
  */
@@ -154,12 +158,37 @@ class CrashLogOSX : public CrashLog {
 		return succeeded;
 	}
 
-	int WriteCrashDump() override
+	bool WriteCrashDump() override
 	{
-		return google_breakpad::ExceptionHandler::WriteMinidump(_personal_dir, MinidumpCallback, this) ? 1 : -1;
+		return google_breakpad::ExceptionHandler::WriteMinidump(_personal_dir, MinidumpCallback, this);
 	}
 #endif
 
+	/* virtual */ bool TryExecute(std::string_view section_name, std::function<bool()> &&func) override
+	{
+		this->try_execute_active = true;
+
+		/* Setup a longjump in case a crash happens. */
+		if (setjmp(this->internal_fault_jmp_buf) != 0) {
+			fmt::print("Something went wrong when attempting to fill {} section of the crash log.\n", section_name);
+
+			/* Reset the signals and continue on. The handler is responsible for dealing with the crash. */
+			sigset_t sigs;
+			sigemptyset(&sigs);
+			for (int signum : _signals_to_handle) {
+				sigaddset(&sigs, signum);
+			}
+			sigprocmask(SIG_UNBLOCK, &sigs, nullptr);
+
+			this->try_execute_active = false;
+			return false;
+		}
+
+		bool res = func();
+		this->try_execute_active = false;
+		return res;
+	}
+
 public:
 	/**
 	 * A crash log is always generated by signal.
@@ -182,52 +211,108 @@ public:
 
 		ShowMacDialog(crash_title, message.c_str(), "Quit");
 	}
+
+	/** Buffer to track the long jump set setup. */
+	jmp_buf internal_fault_jmp_buf;
+
+	/** Whether we are in a TryExecute block. */
+	bool try_execute_active = false;
+
+	/** Points to the current crash log. */
+	static CrashLogOSX *current;
 };
 
-/** The signals we want our crash handler to handle. */
-static const int _signals_to_handle[] = { SIGSEGV, SIGABRT, SIGFPE, SIGBUS, SIGILL, SIGSYS };
+/* static */ CrashLogOSX *CrashLogOSX::current = nullptr;
+
+/**
+ * Set a signal handler for all signals we want to capture.
+ *
+ * @param handler The handler to use.
+ * @return sigset_t A sigset_t containing all signals we want to capture.
+ */
+static sigset_t SetSignals(void(*handler)(int))
+{
+	sigset_t sigs;
+	sigemptyset(&sigs);
+	for (int signum : _signals_to_handle) {
+		sigaddset(&sigs, signum);
+	}
+
+	struct sigaction sa;
+	memset(&sa, 0, sizeof(sa));
+	sa.sa_flags = SA_RESTART;
+
+	sigemptyset(&sa.sa_mask);
+	sa.sa_handler = handler;
+	sa.sa_mask = sigs;
+
+	for (int signum : _signals_to_handle) {
+		sigaction(signum, &sa, nullptr);
+	}
+
+	return sigs;
+}
+
+/**
+ * Entry point for a crash that happened during the handling of a crash.
+ *
+ * @param signum the signal that caused us to crash.
+ */
+static void CDECL HandleInternalCrash(int signum)
+{
+	if (CrashLogOSX::current == nullptr || !CrashLogOSX::current->try_execute_active) {
+		fmt::print("Something went seriously wrong when creating the crash log. Aborting.\n");
+		_exit(1);
+	}
+
+	longjmp(CrashLogOSX::current->internal_fault_jmp_buf, 1);
+}
 
 /**
  * Entry point for the crash handler.
- * @note Not static so it shows up in the backtrace.
+ *
  * @param signum the signal that caused us to crash.
  */
-void CDECL HandleCrash(int signum)
+static void CDECL HandleCrash(int signum)
 {
-	/* Disable all handling of signals by us, so we don't go into infinite loops. */
-	for (const int *i = _signals_to_handle; i != endof(_signals_to_handle); i++) {
-		signal(*i, SIG_DFL);
+	if (CrashLogOSX::current != nullptr) {
+		CrashLog::AfterCrashLogCleanup();
+		_exit(2);
 	}
 
+	/* Capture crashing during the handling of a crash. */
+	sigset_t sigs = SetSignals(HandleInternalCrash);
+	sigset_t old_sigset;
+	sigprocmask(SIG_UNBLOCK, &sigs, &old_sigset);
+
 	if (_gamelog.TestEmergency()) {
 		ShowMacDialog("A serious fault condition occurred in the game. The game will shut down.",
 				"As you loaded an emergency savegame no crash information will be generated.\n",
 				"Quit");
-		abort();
+		_exit(3);
 	}
 
 	if (SaveloadCrashWithMissingNewGRFs()) {
 		ShowMacDialog("A serious fault condition occurred in the game. The game will shut down.",
 				"As you loaded an savegame for which you do not have the required NewGRFs no crash information will be generated.\n",
 				"Quit");
-		abort();
+		_exit(3);
 	}
 
-	CrashLogOSX log(signum);
-	log.MakeCrashLog();
+	CrashLogOSX *log = new CrashLogOSX(signum);
+	CrashLogOSX::current = log;
+	log->MakeCrashLog();
 	if (VideoDriver::GetInstance() == nullptr || VideoDriver::GetInstance()->HasGUI()) {
-		log.DisplayCrashDialog();
+		log->DisplayCrashDialog();
 	}
 
 	CrashLog::AfterCrashLogCleanup();
-	abort();
+	_exit(2);
 }
 
 /* static */ void CrashLog::InitialiseCrashLog()
 {
-	for (const int *i = _signals_to_handle; i != endof(_signals_to_handle); i++) {
-		signal(*i, HandleCrash);
-	}
+	SetSignals(HandleCrash);
 }
 
 /* static */ void CrashLog::InitThread()

src/os/unix/crashlog_unix.cpp

@@ -14,6 +14,7 @@
 #include "../../gamelog.h"
 #include "../../saveload/saveload.h"
 
+#include <setjmp.h>
 #include <signal.h>
 #include <sys/utsname.h>
 
@@ -30,8 +31,17 @@
 #	include <client/linux/handler/exception_handler.h>
 #endif
 
+#if defined(__EMSCRIPTEN__)
+#	include <emscripten.h>
+/* We avoid abort(), as it is a SIGBART, and use _exit() instead. But emscripten doesn't know _exit(). */
+#	define _exit emscripten_force_exit
+#endif
+
 #include "../../safeguards.h"
 
+/** The signals we want our crash handler to handle. */
+static constexpr int _signals_to_handle[] = { SIGSEGV, SIGABRT, SIGFPE, SIGBUS, SIGILL, SIGQUIT };
+
 /**
  * Unix implementation for the crash logger.
  */
@@ -100,12 +110,37 @@ class CrashLogUnix : public CrashLog {
 		return succeeded;
 	}
 
-	int WriteCrashDump() override
+	bool WriteCrashDump() override
 	{
-		return google_breakpad::ExceptionHandler::WriteMinidump(_personal_dir, MinidumpCallback, this) ? 1 : -1;
+		return google_breakpad::ExceptionHandler::WriteMinidump(_personal_dir, MinidumpCallback, this);
 	}
 #endif
 
+	/* virtual */ bool TryExecute(std::string_view section_name, std::function<bool()> &&func) override
+	{
+		this->try_execute_active = true;
+
+		/* Setup a longjump in case a crash happens. */
+		if (setjmp(this->internal_fault_jmp_buf) != 0) {
+			fmt::print("Something went wrong when attempting to fill {} section of the crash log.\n", section_name);
+
+			/* Reset the signals and continue on. The handler is responsible for dealing with the crash. */
+			sigset_t sigs;
+			sigemptyset(&sigs);
+			for (int signum : _signals_to_handle) {
+				sigaddset(&sigs, signum);
+			}
+			sigprocmask(SIG_UNBLOCK, &sigs, nullptr);
+
+			this->try_execute_active = false;
+			return false;
+		}
+
+		bool res = func();
+		this->try_execute_active = false;
+		return res;
+	}
+
 public:
 	/**
 	 * A crash log is always generated by signal.
@@ -115,48 +150,104 @@ public:
 		signum(signum)
 	{
 	}
+
+	/** Buffer to track the long jump set setup. */
+	jmp_buf internal_fault_jmp_buf;
+
+	/** Whether we are in a TryExecute block. */
+	bool try_execute_active = false;
+
+	/** Points to the current crash log. */
+	static CrashLogUnix *current;
 };
 
-/** The signals we want our crash handler to handle. */
-static const int _signals_to_handle[] = { SIGSEGV, SIGABRT, SIGFPE, SIGBUS, SIGILL };
+/* static */ CrashLogUnix *CrashLogUnix::current = nullptr;
+
+/**
+ * Set a signal handler for all signals we want to capture.
+ *
+ * @param handler The handler to use.
+ * @return sigset_t A sigset_t containing all signals we want to capture.
+ */
+static sigset_t SetSignals(void(*handler)(int))
+{
+	sigset_t sigs;
+	sigemptyset(&sigs);
+	for (int signum : _signals_to_handle) {
+		sigaddset(&sigs, signum);
+	}
+
+	struct sigaction sa;
+	memset(&sa, 0, sizeof(sa));
+	sa.sa_flags = SA_RESTART;
+
+	sigemptyset(&sa.sa_mask);
+	sa.sa_handler = handler;
+	sa.sa_mask = sigs;
+
+	for (int signum : _signals_to_handle) {
+		sigaction(signum, &sa, nullptr);
+	}
+
+	return sigs;
+}
+
+/**
+ * Entry point for a crash that happened during the handling of a crash.
+ *
+ * @param signum the signal that caused us to crash.
+ */
+static void CDECL HandleInternalCrash(int signum)
+{
+	if (CrashLogUnix::current == nullptr || !CrashLogUnix::current->try_execute_active) {
+		fmt::print("Something went seriously wrong when creating the crash log. Aborting.\n");
+		_exit(1);
+	}
+
+	longjmp(CrashLogUnix::current->internal_fault_jmp_buf, 1);
+}
 
 /**
  * Entry point for the crash handler.
- * @note Not static so it shows up in the backtrace.
+ *
  * @param signum the signal that caused us to crash.
  */
 static void CDECL HandleCrash(int signum)
 {
-	/* Disable all handling of signals by us, so we don't go into infinite loops. */
-	for (const int *i = _signals_to_handle; i != endof(_signals_to_handle); i++) {
-		signal(*i, SIG_DFL);
+	if (CrashLogUnix::current != nullptr) {
+		CrashLog::AfterCrashLogCleanup();
+		_exit(2);
 	}
 
+	/* Capture crashing during the handling of a crash. */
+	sigset_t sigs = SetSignals(HandleInternalCrash);
+	sigset_t old_sigset;
+	sigprocmask(SIG_UNBLOCK, &sigs, &old_sigset);
+
 	if (_gamelog.TestEmergency()) {
 		fmt::print("A serious fault condition occurred in the game. The game will shut down.\n");
 		fmt::print("As you loaded an emergency savegame no crash information will be generated.\n");
-		abort();
+		_exit(3);
 	}
 
 	if (SaveloadCrashWithMissingNewGRFs()) {
 		fmt::print("A serious fault condition occurred in the game. The game will shut down.\n");
 		fmt::print("As you loaded an savegame for which you do not have the required NewGRFs\n");
 		fmt::print("no crash information will be generated.\n");
-		abort();
+		_exit(3);
 	}
 
-	CrashLogUnix log(signum);
-	log.MakeCrashLog();
+	CrashLogUnix *log = new CrashLogUnix(signum);
+	CrashLogUnix::current = log;
+	log->MakeCrashLog();
 
 	CrashLog::AfterCrashLogCleanup();
-	abort();
+	_exit(2);
 }
 
 /* static */ void CrashLog::InitialiseCrashLog()
 {
-	for (const int *i = _signals_to_handle; i != endof(_signals_to_handle); i++) {
-		signal(*i, HandleCrash);
-	}
+	SetSignals(HandleCrash);
 }
 
 /* static */ void CrashLog::InitThread()

src/os/windows/crashlog_win.cpp

@@ -25,6 +25,8 @@
 
 #if defined(_MSC_VER)
 #	include <dbghelp.h>
+#else
+#	include <setjmp.h>
 #endif
 
 #ifdef WITH_UNOFFICIAL_BREAKPAD
@@ -33,6 +35,21 @@
 
 #include "../../safeguards.h"
 
+/** Exception code used for custom abort. */
+static constexpr DWORD CUSTOM_ABORT_EXCEPTION = 0xE1212012;
+
+/**
+ * Forcefully try to terminate the application.
+ *
+ * @param exit_code The exit code to return.
+ */
+static void NORETURN ImmediateExitProcess(uint exit_code)
+{
+	/* TerminateProcess may fail in some special edge cases; fall back to ExitProcess in this case. */
+	TerminateProcess(GetCurrentProcess(), exit_code);
+	ExitProcess(exit_code);
+}
+
 /**
  * Windows implementation for the crash logger.
  */
@@ -55,21 +72,65 @@ public:
 		return succeeded;
 	}
 
-	int WriteCrashDump() override
+	bool WriteCrashDump() override
 	{
-		return google_breakpad::ExceptionHandler::WriteMinidump(OTTD2FS(_personal_dir), MinidumpCallback, this) ? 1 : -1;
+		return google_breakpad::ExceptionHandler::WriteMinidump(OTTD2FS(_personal_dir), MinidumpCallback, this);
 	}
 #endif
 
+#if defined(_MSC_VER)
+	/* virtual */ bool TryExecute(std::string_view section_name, std::function<bool()> &&func) override
+	{
+		this->try_execute_active = true;
+		bool res;
+
+		__try {
+			res = func();
+		} __except (EXCEPTION_EXECUTE_HANDLER) {
+			fmt::print("Something went wrong when attempting to fill {} section of the crash log.\n", section_name);
+			res = false;
+		}
+
+		this->try_execute_active = false;
+		return res;
+	}
+#else
+	/* virtual */ bool TryExecute(std::string_view section_name, std::function<bool()> &&func) override
+	{
+		this->try_execute_active = true;
+
+		/* Setup a longjump in case a crash happens. */
+		if (setjmp(this->internal_fault_jmp_buf) != 0) {
+			fmt::print("Something went wrong when attempting to fill {} section of the crash log.\n", section_name);
+
+			this->try_execute_active = false;
+			return false;
+		}
+
+		bool res = func();
+		this->try_execute_active = false;
+		return res;
+	}
+#endif /* _MSC_VER */
+
 	/**
 	 * A crash log is always generated when it's generated.
 	 * @param ep the data related to the exception.
 	 */
-	CrashLogWindows(EXCEPTION_POINTERS *ep = nullptr) : ep(ep) {}
+	CrashLogWindows(EXCEPTION_POINTERS *ep = nullptr) :
+		ep(ep)
+	{
+	}
 
-	/**
-	 * Points to the current crash log.
-	 */
+#if !defined(_MSC_VER)
+	/** Buffer to track the long jump set setup. */
+	jmp_buf internal_fault_jmp_buf;
+#endif
+
+	/** Whether we are in a TryExecute block. */
+	bool try_execute_active = false;
+
+	/** Points to the current crash log. */
 	static CrashLogWindows *current;
 };
 
@@ -248,7 +309,7 @@ static LONG WINAPI ExceptionHandler(EXCEPTION_POINTERS *ep)
 
 	if (CrashLogWindows::current != nullptr) {
 		CrashLog::AfterCrashLogCleanup();
-		ExitProcess(2);
+		ImmediateExitProcess(2);
 	}
 
 	if (_gamelog.TestEmergency()) {
@@ -256,7 +317,7 @@ static LONG WINAPI ExceptionHandler(EXCEPTION_POINTERS *ep)
 			L"A serious fault condition occurred in the game. The game will shut down.\n"
 			L"As you loaded an emergency savegame no crash information will be generated.\n";
 		MessageBox(nullptr, _emergency_crash, L"Fatal Application Failure", MB_ICONERROR);
-		ExitProcess(3);
+		ImmediateExitProcess(3);
 	}
 
 	if (SaveloadCrashWithMissingNewGRFs()) {
@@ -265,7 +326,7 @@ static LONG WINAPI ExceptionHandler(EXCEPTION_POINTERS *ep)
 			L"As you loaded an savegame for which you do not have the required NewGRFs\n"
 			L"no crash information will be generated.\n";
 		MessageBox(nullptr, _saveload_crash, L"Fatal Application Failure", MB_ICONERROR);
-		ExitProcess(3);
+		ImmediateExitProcess(3);
 	}
 
 	CrashLogWindows *log = new CrashLogWindows(ep);
@@ -293,9 +354,32 @@ static LONG WINAPI ExceptionHandler(EXCEPTION_POINTERS *ep)
 	return EXCEPTION_EXECUTE_HANDLER;
 }
 
+static LONG WINAPI VectoredExceptionHandler(EXCEPTION_POINTERS *ep)
+{
+	if (CrashLogWindows::current != nullptr && CrashLogWindows::current->try_execute_active) {
+#if defined(_MSC_VER)
+		return EXCEPTION_CONTINUE_SEARCH;
+#else
+		longjmp(CrashLogWindows::current->internal_fault_jmp_buf, 1);
+#endif
+	}
+
+	if (ep->ExceptionRecord->ExceptionCode == 0xC0000374 /* heap corruption */) {
+		return ExceptionHandler(ep);
+	}
+	if (ep->ExceptionRecord->ExceptionCode == EXCEPTION_STACK_OVERFLOW) {
+		return ExceptionHandler(ep);
+	}
+	if (ep->ExceptionRecord->ExceptionCode == CUSTOM_ABORT_EXCEPTION) {
+		return ExceptionHandler(ep);
+	}
+
+	return EXCEPTION_CONTINUE_SEARCH;
+}
+
 static void CDECL CustomAbort(int signal)
 {
-	RaiseException(0xE1212012, 0, 0, nullptr);
+	RaiseException(CUSTOM_ABORT_EXCEPTION, 0, 0, nullptr);
 }
 
 /* static */ void CrashLog::InitialiseCrashLog()
@@ -309,6 +393,7 @@ static void CDECL CustomAbort(int signal)
 	_set_abort_behavior(0, _WRITE_ABORT_MSG);
 #endif
 	SetUnhandledExceptionFilter(ExceptionHandler);
+	AddVectoredExceptionHandler(1, VectoredExceptionHandler);
 }
 
 /* static */ void CrashLog::InitThread()
@@ -422,7 +507,7 @@ static INT_PTR CALLBACK CrashDialogFunc(HWND wnd, UINT msg, WPARAM wParam, LPARA
 			switch (wParam) {
 				case 12: // Close
 					CrashLog::AfterCrashLogCleanup();
-					ExitProcess(2);
+					ImmediateExitProcess(2);
 				case 15: // Expand window to show crash-message
 					_expanded = !_expanded;
 					SetWndSize(wnd, _expanded);
@@ -431,7 +516,7 @@ static INT_PTR CALLBACK CrashDialogFunc(HWND wnd, UINT msg, WPARAM wParam, LPARA
 			return TRUE;
 		case WM_CLOSE:
 			CrashLog::AfterCrashLogCleanup();
-			ExitProcess(2);
+			ImmediateExitProcess(2);
 	}
 
 	return FALSE;