From 1100b04b59b461f1a2cc3dfe7b53b5473daa7992 Mon Sep 17 00:00:00 2001 From: Tiger Wang Date: Wed, 21 Apr 2021 16:07:48 +0100 Subject: Make Windows go brrrr, not tick. tick. tick. (#5201) * Fixes #5140 --- src/OSSupport/CMakeLists.txt | 4 +- src/OSSupport/ConsoleSignalHandler.h | 130 +++++++++++++++++++++++++++++++ src/OSSupport/Errors.cpp | 53 ------------- src/OSSupport/Errors.h | 5 -- src/OSSupport/Event.cpp | 1 - src/OSSupport/MiniDumpWriter.h | 137 +++++++++++++++++---------------- src/OSSupport/SleepResolutionBooster.h | 66 ++++++++++++++++ src/OSSupport/StartAsService.h | 20 +++-- 8 files changed, 278 insertions(+), 138 deletions(-) create mode 100644 src/OSSupport/ConsoleSignalHandler.h delete mode 100644 src/OSSupport/Errors.cpp delete mode 100644 src/OSSupport/Errors.h create mode 100644 src/OSSupport/SleepResolutionBooster.h (limited to 'src/OSSupport') diff --git a/src/OSSupport/CMakeLists.txt b/src/OSSupport/CMakeLists.txt index 42ce6934d..742bdcccf 100644 --- a/src/OSSupport/CMakeLists.txt +++ b/src/OSSupport/CMakeLists.txt @@ -2,7 +2,6 @@ target_sources( ${CMAKE_PROJECT_NAME} PRIVATE CriticalSection.cpp - Errors.cpp Event.cpp File.cpp GZipFile.cpp @@ -19,8 +18,8 @@ target_sources( WinStackWalker.cpp AtomicUniquePtr.h + ConsoleSignalHandler.h CriticalSection.h - Errors.h Event.h File.h GetAddressInfoError.h @@ -34,6 +33,7 @@ target_sources( NetworkSingleton.h Queue.h ServerHandleImpl.h + SleepResolutionBooster.h StackTrace.h StartAsService.h TCPLinkImpl.h diff --git a/src/OSSupport/ConsoleSignalHandler.h b/src/OSSupport/ConsoleSignalHandler.h new file mode 100644 index 000000000..23e63d555 --- /dev/null +++ b/src/OSSupport/ConsoleSignalHandler.h @@ -0,0 +1,130 @@ + +// ConsoleSignalHandler.h + +// Intercepts signals for graceful CTRL-C (and others) handling. + +// This file MUST NOT be included from anywhere other than main.cpp. + + + + + +#include + + + + + +// Because SIG_DFL or SIG_IGN could be NULL instead of nullptr, we need to disable the Clang warning here: +#ifdef __clang__ +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wunknown-warning-option" +#pragma clang diagnostic ignored "-Wunknown-pragmas" +#pragma clang diagnostic ignored "-Wzero-as-null-pointer-constant" +#endif + +static void NonCtrlHandler(int a_Signal) +{ + LOGD("Terminate event raised from std::signal"); + + switch (a_Signal) + { + case SIGSEGV: + { + PrintStackTrace(); + + LOGERROR( + "Failure report: \n\n" + " :( | Cuberite has encountered an error and needs to close\n" + " | SIGSEGV: Segmentation fault\n" + " |\n" +#ifdef BUILD_ID + " | Cuberite " BUILD_SERIES_NAME " (id: " BUILD_ID ")\n" + " | from commit " BUILD_COMMIT_ID "\n" +#endif + ); + + std::signal(SIGSEGV, SIG_DFL); + return; + } + case SIGABRT: +#ifdef SIGABRT_COMPAT + case SIGABRT_COMPAT: +#endif + { + PrintStackTrace(); + + LOGERROR( + "Failure report: \n\n" + " :( | Cuberite has encountered an error and needs to close\n" + " | SIGABRT: Server self-terminated due to an internal fault\n" + " |\n" +#ifdef BUILD_ID + " | Cuberite " BUILD_SERIES_NAME " (id: " BUILD_ID ")\n" + " | from commit " BUILD_COMMIT_ID "\n" +#endif + ); + + std::signal(SIGSEGV, SIG_DFL); + return; + } + case SIGINT: + case SIGTERM: + { + // Server is shutting down, wait for it... + cRoot::Stop(); + return; + } + } +} + +#ifdef __clang__ +#pragma clang diagnostic pop +#endif + + + + + +#ifdef _WIN32 + +/** Handle CTRL events in windows, including console window close. */ +static BOOL CtrlHandler(DWORD fdwCtrlType) +{ + cRoot::Stop(); + LOGD("Terminate event raised from the Windows CtrlHandler"); + + // Delay as much as possible to try to get the server to shut down cleanly - 10 seconds given by Windows: + std::this_thread::sleep_for(std::chrono::seconds(10)); + + // Returning from main() automatically aborts this handler thread. + + return TRUE; +} + +#endif + + + + + +namespace ConsoleSignalHandler +{ + static void Register() + { + std::signal(SIGSEGV, NonCtrlHandler); + std::signal(SIGTERM, NonCtrlHandler); + std::signal(SIGINT, NonCtrlHandler); + std::signal(SIGABRT, NonCtrlHandler); +#ifdef SIGABRT_COMPAT + std::signal(SIGABRT_COMPAT, NonCtrlHandler); +#endif +#ifdef SIGPIPE + std::signal(SIGPIPE, SIG_IGN); // Ignore (PR #2487). +#endif + +#ifdef _WIN32 + SetConsoleCtrlHandler(reinterpret_cast(CtrlHandler), TRUE); +#endif + } +}; diff --git a/src/OSSupport/Errors.cpp b/src/OSSupport/Errors.cpp deleted file mode 100644 index df94d34da..000000000 --- a/src/OSSupport/Errors.cpp +++ /dev/null @@ -1,53 +0,0 @@ - -#include "Globals.h" - -#include "Errors.h" - -AString GetOSErrorString( int a_ErrNo) -{ - char buffer[ 1024 ]; - AString Out; - - #ifdef _WIN32 - - FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM, nullptr, a_ErrNo, 0, buffer, ARRAYCOUNT(buffer), nullptr); - Printf(Out, "%d: %s", a_ErrNo, buffer); - if (!Out.empty() && (Out[Out.length() - 1] == '\n')) - { - Out.erase(Out.length() - 2); - } - return Out; - - #else // _WIN32 - - // According to https://linux.die.net/man/3/strerror_r there are two versions of strerror_r(): - - #if defined(__GLIBC__) && defined( _GNU_SOURCE) // GNU version of strerror_r() - - char * res = strerror_r( errno, buffer, ARRAYCOUNT(buffer)); - if (res != nullptr) - { - Printf(Out, "%d: %s", a_ErrNo, res); - return Out; - } - - #else // XSI version of strerror_r(): - - int res = strerror_r( errno, buffer, ARRAYCOUNT(buffer)); - if (res == 0) - { - Printf(Out, "%d: %s", a_ErrNo, buffer); - return Out; - } - - #endif // strerror_r() version - - else - { - Printf(Out, "Error %d while getting error string for error #%d!", errno, a_ErrNo); - return Out; - } - - #endif // else _WIN32 -} - diff --git a/src/OSSupport/Errors.h b/src/OSSupport/Errors.h deleted file mode 100644 index 8ce9deb10..000000000 --- a/src/OSSupport/Errors.h +++ /dev/null @@ -1,5 +0,0 @@ - -#pragma once - -AString GetOSErrorString(int a_ErrNo); - diff --git a/src/OSSupport/Event.cpp b/src/OSSupport/Event.cpp index 4a16ddee7..623357766 100644 --- a/src/OSSupport/Event.cpp +++ b/src/OSSupport/Event.cpp @@ -7,7 +7,6 @@ #include "Globals.h" // NOTE: MSVC stupidness requires this to be the same across all modules #include "Event.h" -#include "Errors.h" diff --git a/src/OSSupport/MiniDumpWriter.h b/src/OSSupport/MiniDumpWriter.h index e5cd1a0ac..c223fa9fb 100644 --- a/src/OSSupport/MiniDumpWriter.h +++ b/src/OSSupport/MiniDumpWriter.h @@ -1,5 +1,11 @@ -#pragma once +// MiniDumpWriter.h + +// 32-bit only: +// When the server crashes, create a "dump file" containing the callstack of each thread and some variables; +// let the user send us that crash file for analysis. + +// This file MUST NOT be included from anywhere other than main.cpp. @@ -18,30 +24,64 @@ enum class MiniDumpFlags #if defined(_WIN32) && !defined(_WIN64) && defined(_MSC_VER) // 32-bit Windows app compiled in MSVC -#include +#include + + + + + +using MiniDumpWriteDumpFunction = decltype(&MiniDumpWriteDump); + +static HINSTANCE m_DbgHelp; +static MiniDumpWriteDumpFunction s_WriteMiniDump; // The function in dbghlp DLL that creates dump files +static wchar_t s_DumpFileName[MAX_PATH]; // Filename of the dump file; hes to be created before the dump handler kicks in +static char s_ExceptionStack[128 * 1024]; // Substitute stack, just in case the handler kicks in because of "insufficient stack space" +static MINIDUMP_TYPE s_DumpFlags = MiniDumpNormal; // By default dump only the stack and some helpers + + + + + +/** This function gets called just before the "program executed an illegal instruction and will be terminated" or similar. +Its purpose is to create the crashdump using the dbghlp DLLs */ +static LONG WINAPI LastChanceExceptionFilter(__in struct _EXCEPTION_POINTERS * a_ExceptionInfo) +{ + char * newStack = &s_ExceptionStack[sizeof(s_ExceptionStack) - 1]; + char * oldStack; + + // Use the substitute stack: + _asm + { + mov oldStack, esp + mov esp, newStack + } + + MINIDUMP_EXCEPTION_INFORMATION ExcInformation; + ExcInformation.ThreadId = GetCurrentThreadId(); + ExcInformation.ExceptionPointers = a_ExceptionInfo; + ExcInformation.ClientPointers = 0; + + // Write the dump file: + HANDLE dumpFile = CreateFile(s_DumpFileName, GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr); + s_WriteMiniDump(GetCurrentProcess(), GetCurrentProcessId(), dumpFile, s_DumpFlags, (a_ExceptionInfo) ? &ExcInformation : nullptr, nullptr, nullptr); + CloseHandle(dumpFile); + + // Revert to old stack: + _asm + { + mov esp, oldStack + } + + return 0; +} -/** Windows 32-bit stuff: -When the server crashes, create a "dump file" containing the callstack of each thread and some variables; -let the user send us that crash file for analysis */ -class MiniDumpWriter +namespace MiniDumpWriter { - typedef BOOL(WINAPI *pMiniDumpWriteDump)( - HANDLE hProcess, - DWORD ProcessId, - HANDLE hFile, - MINIDUMP_TYPE DumpType, - PMINIDUMP_EXCEPTION_INFORMATION ExceptionParam, - PMINIDUMP_USER_STREAM_INFORMATION UserStreamParam, - PMINIDUMP_CALLBACK_INFORMATION CallbackParam - ); - -public: - - MiniDumpWriter() + static void Register() { // Magic code to produce dump-files on Windows if the server crashes: @@ -51,7 +91,7 @@ public: return; } - s_WriteMiniDump = (pMiniDumpWriteDump)GetProcAddress(m_DbgHelp, "MiniDumpWriteDump"); + s_WriteMiniDump = (MiniDumpWriteDumpFunction)GetProcAddress(m_DbgHelp, "MiniDumpWriteDump"); if (s_WriteMiniDump != nullptr) { ASSERT(swprintf(s_DumpFileName, ARRAYCOUNT(s_DumpFileName), L"crash_mcs_%x.dmp", GetCurrentProcessId()) > 0); @@ -61,7 +101,7 @@ public: // End of dump-file magic } - void AddDumpFlags(const MiniDumpFlags a_Flags) + static void AddDumpFlags(const MiniDumpFlags a_Flags) { switch (a_Flags) { @@ -78,60 +118,25 @@ public: } } - ~MiniDumpWriter() + static void Unregister() { FreeLibrary(m_DbgHelp); } +}; -private: +#else - /** This function gets called just before the "program executed an illegal instruction and will be terminated" or similar. - Its purpose is to create the crashdump using the dbghlp DLLs */ - static LONG WINAPI LastChanceExceptionFilter(__in struct _EXCEPTION_POINTERS * a_ExceptionInfo) +namespace MiniDumpWriter +{ + static void Register() { - char * newStack = &s_ExceptionStack[sizeof(s_ExceptionStack) - 1]; - char * oldStack; - - // Use the substitute stack: - // This code is the reason why we don't support 64-bit (yet) - _asm - { - mov oldStack, esp - mov esp, newStack - } - - MINIDUMP_EXCEPTION_INFORMATION ExcInformation; - ExcInformation.ThreadId = GetCurrentThreadId(); - ExcInformation.ExceptionPointers = a_ExceptionInfo; - ExcInformation.ClientPointers = 0; - - // Write the dump file: - HANDLE dumpFile = CreateFile(s_DumpFileName, GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr); - s_WriteMiniDump(GetCurrentProcess(), GetCurrentProcessId(), dumpFile, s_DumpFlags, (a_ExceptionInfo) ? &ExcInformation : nullptr, nullptr, nullptr); - CloseHandle(dumpFile); - - // Revert to old stack: - _asm - { - mov esp, oldStack - } - - return 0; } - HINSTANCE m_DbgHelp; - - static inline pMiniDumpWriteDump s_WriteMiniDump; // The function in dbghlp DLL that creates dump files - static inline wchar_t s_DumpFileName[MAX_PATH]; // Filename of the dump file; hes to be created before the dump handler kicks in - static inline char s_ExceptionStack[128 * 1024]; // Substitute stack, just in case the handler kicks in because of "insufficient stack space" - static inline MINIDUMP_TYPE s_DumpFlags = MiniDumpNormal; // By default dump only the stack and some helpers -}; - -#else + static void AddDumpFlags(const MiniDumpFlags) + { + } -struct MiniDumpWriter -{ - void AddDumpFlags(const MiniDumpFlags) + static void Unregister() { } }; diff --git a/src/OSSupport/SleepResolutionBooster.h b/src/OSSupport/SleepResolutionBooster.h new file mode 100644 index 000000000..f090f6963 --- /dev/null +++ b/src/OSSupport/SleepResolutionBooster.h @@ -0,0 +1,66 @@ + +// SleepResolutionBooster.h + +// Increases the accuracy of Sleep on Windows (GH #5140). + +// This file MUST NOT be included from anywhere other than main.cpp. + + + + + +#ifdef _WIN32 + +#include + + + + + +static TIMECAPS g_Resolution; + + + + + +namespace SleepResolutionBooster +{ + static void Register() + { + // Default sleep resolution on Windows isn't accurate enough (GH #5140) so try to boost it: + if ( + (timeGetDevCaps(&g_Resolution, sizeof(g_Resolution)) == MMSYSERR_NOERROR) && + (timeBeginPeriod(g_Resolution.wPeriodMin) == MMSYSERR_NOERROR) + ) + { + return; + } + + // Max < Min sentinel for failure, to prevent bogus timeEndPeriod calls: + g_Resolution.wPeriodMax = 0; + g_Resolution.wPeriodMin = 1; + } + + static void Unregister() + { + if (g_Resolution.wPeriodMax >= g_Resolution.wPeriodMin) + { + timeEndPeriod(g_Resolution.wPeriodMin); + } + } +}; + +#else + +namespace SleepResolutionBooster +{ + static void Register() + { + } + + static void Unregister() + { + } +}; + +#endif diff --git a/src/OSSupport/StartAsService.h b/src/OSSupport/StartAsService.h index 472184367..fe078f303 100644 --- a/src/OSSupport/StartAsService.h +++ b/src/OSSupport/StartAsService.h @@ -1,19 +1,17 @@ -#pragma once +// StartAsService.h +// Handles startup as a Windows Service or UNIX daemon. +// This file MUST NOT be included from anywhere other than main.cpp. -#ifdef _WIN32 - -#include - - +#ifdef _WIN32 -class cStartAsService +class StartAsService { public: @@ -79,7 +77,7 @@ private: return; } - const auto LastComponent = wcsrchr(applicationFilename, L'\\'); + const auto LastComponent = std::wcsrchr(applicationFilename, L'\\'); if (LastComponent == nullptr) { serviceSetState(0, SERVICE_STOPPED, E_UNEXPECTED); @@ -89,7 +87,7 @@ private: const auto LengthToLastComponent = LastComponent - applicationFilename; // Strip off the filename, keep only the path: - wcsncpy(applicationDirectory, applicationFilename, LengthToLastComponent); + std::wcsncpy(applicationDirectory, applicationFilename, LengthToLastComponent); applicationDirectory[LengthToLastComponent] = L'\0'; // Make sure new path is null terminated // Services are run by the SCM, and inherit its working directory - usually System32. @@ -115,7 +113,7 @@ private: char * MultibyteArgV[] = { MultibyteArgV0 }; const auto OutputSize = std::size(MultibyteArgV0); - const auto TranslateResult = wcstombs(MultibyteArgV0, argv[0], OutputSize); + const auto TranslateResult = std::wcstombs(MultibyteArgV0, argv[0], OutputSize); if (TranslateResult == static_cast(-1)) { @@ -141,7 +139,7 @@ private: #else -struct cStartAsService +struct StartAsService { /** Make a UNIX daemon. */ template -- cgit v1.2.3