From 3b48d8c9d662e829499a01f990e00dd27a0ffcdc Mon Sep 17 00:00:00 2001 From: Murat Seker Date: Sun, 12 Jul 2020 18:27:24 +0200 Subject: [PATCH] Dump stack trace on crash (Implement #849) (#870) * Print stack trace on crash * Printing stack trace is fully functional except for WIN32 * Minor fixes * Minor fixes --- include/grabber/V4L2Wrapper.h | 1 + include/grabber/X11Wrapper.h | 5 + include/utils/DefaultSignalHandler.h | 6 + libsrc/grabber/dispmanx/DispmanxWrapper.cpp | 2 +- libsrc/grabber/v4l2/V4L2Wrapper.cpp | 5 + libsrc/grabber/x11/X11Wrapper.cpp | 8 ++ libsrc/hyperion/GrabberWrapper.cpp | 13 +- libsrc/hyperion/Hyperion.cpp | 15 +- libsrc/leddevice/LedDevice.cpp | 2 +- libsrc/leddevice/dev_spi/ProviderSpi.cpp | 2 +- libsrc/ssdp/SSDPHandler.cpp | 21 ++- libsrc/utils/DefaultSignalHandler.cpp | 132 ++++++++++++++++++ src/hyperion-aml/hyperion-aml.cpp | 4 + src/hyperion-dispmanx/hyperion-dispmanx.cpp | 4 + .../hyperion-framebuffer.cpp | 4 + src/hyperion-osx/hyperion-osx.cpp | 4 + src/hyperion-remote/hyperion-remote.cpp | 3 + src/hyperion-v4l2/hyperion-v4l2.cpp | 2 + src/hyperion-x11/hyperion-x11.cpp | 3 + src/hyperiond/detectProcess.h | 70 ++++------ src/hyperiond/main.cpp | 19 +-- src/hyperiond/systray.cpp | 2 +- 22 files changed, 239 insertions(+), 88 deletions(-) create mode 100644 include/utils/DefaultSignalHandler.h create mode 100644 libsrc/utils/DefaultSignalHandler.cpp diff --git a/include/grabber/V4L2Wrapper.h b/include/grabber/V4L2Wrapper.h index fc9596a6..f614f642 100644 --- a/include/grabber/V4L2Wrapper.h +++ b/include/grabber/V4L2Wrapper.h @@ -16,6 +16,7 @@ public: VideoStandard videoStandard, PixelFormat pixelFormat, int pixelDecimation ); + ~V4L2Wrapper() override; bool getSignalDetectionEnable(); diff --git a/include/grabber/X11Wrapper.h b/include/grabber/X11Wrapper.h index 9271d9cd..93dc1e5e 100644 --- a/include/grabber/X11Wrapper.h +++ b/include/grabber/X11Wrapper.h @@ -26,6 +26,11 @@ public: /// X11Wrapper(int cropLeft, int cropRight, int cropTop, int cropBottom, int pixelDecimation, const unsigned updateRate_Hz); + /// + /// Destructor of this framebuffer frame grabber. Releases any claimed resources. + /// + ~X11Wrapper() override; + public slots: /// /// Performs a single frame grab and computes the led-colors diff --git a/include/utils/DefaultSignalHandler.h b/include/utils/DefaultSignalHandler.h new file mode 100644 index 00000000..8d063e13 --- /dev/null +++ b/include/utils/DefaultSignalHandler.h @@ -0,0 +1,6 @@ +#pragma once + +namespace DefaultSignalHandler +{ + void install(); +} diff --git a/libsrc/grabber/dispmanx/DispmanxWrapper.cpp b/libsrc/grabber/dispmanx/DispmanxWrapper.cpp index 0acf81d8..d6b472e8 100644 --- a/libsrc/grabber/dispmanx/DispmanxWrapper.cpp +++ b/libsrc/grabber/dispmanx/DispmanxWrapper.cpp @@ -4,7 +4,7 @@ DispmanxWrapper::DispmanxWrapper(const unsigned grabWidth, const unsigned grabHe : GrabberWrapper("Dispmanx", &_grabber, grabWidth, grabHeight, updateRate_Hz) , _grabber(grabWidth, grabHeight) { - + } void DispmanxWrapper::action() diff --git a/libsrc/grabber/v4l2/V4L2Wrapper.cpp b/libsrc/grabber/v4l2/V4L2Wrapper.cpp index ec420ea4..5f75740f 100644 --- a/libsrc/grabber/v4l2/V4L2Wrapper.cpp +++ b/libsrc/grabber/v4l2/V4L2Wrapper.cpp @@ -33,6 +33,11 @@ V4L2Wrapper::V4L2Wrapper(const QString &device, connect(&_grabber, SIGNAL(readError(const char*)), this, SLOT(readError(const char*)), Qt::DirectConnection); } +V4L2Wrapper::~V4L2Wrapper() +{ + stop(); +} + bool V4L2Wrapper::start() { return ( _grabber.start() && GrabberWrapper::start()); diff --git a/libsrc/grabber/x11/X11Wrapper.cpp b/libsrc/grabber/x11/X11Wrapper.cpp index ee445ea2..91f2f8f7 100644 --- a/libsrc/grabber/x11/X11Wrapper.cpp +++ b/libsrc/grabber/x11/X11Wrapper.cpp @@ -6,6 +6,14 @@ X11Wrapper::X11Wrapper(int cropLeft, int cropRight, int cropTop, int cropBottom, , _init(false) {} +X11Wrapper::~X11Wrapper() +{ + if ( _init ) + { + stop(); + } +} + void X11Wrapper::action() { if (! _init ) diff --git a/libsrc/hyperion/GrabberWrapper.cpp b/libsrc/hyperion/GrabberWrapper.cpp index f7f0b039..394576b7 100644 --- a/libsrc/hyperion/GrabberWrapper.cpp +++ b/libsrc/hyperion/GrabberWrapper.cpp @@ -39,7 +39,7 @@ GrabberWrapper::GrabberWrapper(QString grabberName, Grabber * ggrabber, unsigned GrabberWrapper::~GrabberWrapper() { - GrabberWrapper::stop(); // TODO Is this right???????? + stop(); Debug(_log,"Close grabber: %s", QSTRING_CSTR(_grabberName)); } @@ -53,9 +53,12 @@ bool GrabberWrapper::start() void GrabberWrapper::stop() { - // Stop the timer, effectivly stopping the process - Debug(_log,"Grabber stop()"); - _timer->stop(); + if (_timer->isActive()) + { + // Stop the timer, effectivly stopping the process + Debug(_log,"Grabber stop()"); + _timer->stop(); + } } QStringList GrabberWrapper::availableGrabbers() @@ -216,7 +219,7 @@ void GrabberWrapper::handleSourceRequest(const hyperion::Components& component, void GrabberWrapper::tryStart() { - // verify start condition + // verify start condition if((_grabberName.startsWith("V4L") && !GRABBER_V4L_CLIENTS.empty()) || (!_grabberName.startsWith("V4L") && !GRABBER_SYS_CLIENTS.empty())) { start(); diff --git a/libsrc/hyperion/Hyperion.cpp b/libsrc/hyperion/Hyperion.cpp index 32cd10e6..fb422018 100644 --- a/libsrc/hyperion/Hyperion.cpp +++ b/libsrc/hyperion/Hyperion.cpp @@ -141,9 +141,8 @@ void Hyperion::start() _boblightServer = new BoblightServer(this, getSetting(settings::BOBLSERVER)); connect(this, &Hyperion::settingsChanged, _boblightServer, &BoblightServer::handleSettingsUpdate); - // instance inited + // instance inited, enter thread event loop emit started(); - // enter thread event loop } void Hyperion::stop() @@ -380,18 +379,8 @@ void Hyperion::setColor(const int priority, const std::vector &ledColo _effectEngine->channelCleared(priority); // create full led vector from single/multiple colors - size_t size = _ledString.leds().size(); std::vector newLedColors; - while (true) - { - for (const auto &entry : ledColors) - { - newLedColors.emplace_back(entry); - if (newLedColors.size() == size) - goto end; - } - } -end: + std::copy_n(ledColors.begin(), _ledString.leds().size(), std::back_inserter(newLedColors)); if (getPriorityInfo(priority).componentId != hyperion::COMP_COLOR) clear(priority); diff --git a/libsrc/leddevice/LedDevice.cpp b/libsrc/leddevice/LedDevice.cpp index 5c831ba5..3ef2e63d 100644 --- a/libsrc/leddevice/LedDevice.cpp +++ b/libsrc/leddevice/LedDevice.cpp @@ -36,7 +36,7 @@ LedDevice::LedDevice(const QJsonObject& config, QObject* parent) LedDevice::~LedDevice() { - _refresh_timer->deleteLater(); + delete _refresh_timer; } int LedDevice::open() diff --git a/libsrc/leddevice/dev_spi/ProviderSpi.cpp b/libsrc/leddevice/dev_spi/ProviderSpi.cpp index d4082bf9..008afaa7 100644 --- a/libsrc/leddevice/dev_spi/ProviderSpi.cpp +++ b/libsrc/leddevice/dev_spi/ProviderSpi.cpp @@ -39,7 +39,7 @@ bool ProviderSpi::init(const QJsonObject &deviceConfig) _baudRate_Hz = deviceConfig["rate"].toInt(_baudRate_Hz); _spiMode = deviceConfig["spimode"].toInt(_spiMode); _spiDataInvert = deviceConfig["invert"].toBool(_spiDataInvert); - + return isInitOK; } diff --git a/libsrc/ssdp/SSDPHandler.cpp b/libsrc/ssdp/SSDPHandler.cpp index 4ee6dfb0..a8d8bd7c 100644 --- a/libsrc/ssdp/SSDPHandler.cpp +++ b/libsrc/ssdp/SSDPHandler.cpp @@ -116,20 +116,17 @@ void SSDPHandler::handleWebServerStateChange(const bool newState) void SSDPHandler::handleNetworkConfigurationChanged(const QNetworkConfiguration &config) { // get localAddress from interface - if(!getLocalAddress().isEmpty()) + QString localAddress = getLocalAddress(); + if(!localAddress.isEmpty() && _localAddress != localAddress) { - QString localAddress = getLocalAddress(); - if(_localAddress != localAddress) - { - // revoke old ip - sendAnnounceList(false); + // revoke old ip + sendAnnounceList(false); - // update desc & notify new ip - _localAddress = localAddress; - _webserver->setSSDPDescription(buildDesc()); - setDescriptionAddress(getDescAddress()); - sendAnnounceList(true); - } + // update desc & notify new ip + _localAddress = localAddress; + _webserver->setSSDPDescription(buildDesc()); + setDescriptionAddress(getDescAddress()); + sendAnnounceList(true); } } diff --git a/libsrc/utils/DefaultSignalHandler.cpp b/libsrc/utils/DefaultSignalHandler.cpp new file mode 100644 index 00000000..4fec4596 --- /dev/null +++ b/libsrc/utils/DefaultSignalHandler.cpp @@ -0,0 +1,132 @@ +#ifndef _WIN32 + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +namespace DefaultSignalHandler +{ + +std::string decipher_trace(const std::string &trace) +{ + std::string result; + + if(trace.empty()) + { + result += "??\n"; + return result; + } + + auto* begin = strchr(trace.c_str(), '(') + 1; + auto* end = strchr(begin, '+'); + + if(!end) + end = strchr(begin, ')'); + + std::string mangled_name(begin, end); + + int status; + char * realname = abi::__cxa_demangle(mangled_name.c_str(), 0, 0, &status); + result.insert(result.end(), trace.c_str(), begin); + + if(realname) + result += realname; + else + result.insert(result.end(), begin, end); + + free(realname); + result.insert(result.size(), end); + + return result; +} + +void print_trace() +{ + const int MAX_SIZE = 50; + void * addresses[MAX_SIZE]; + int size = backtrace(addresses, MAX_SIZE); + + if (!size) + return; + + Logger* log = Logger::getInstance("CORE"); + char ** symbols = backtrace_symbols(addresses, size); + for (int i = 0; i < size; ++i) + { + std::string line = "\t" + decipher_trace(symbols[i]); + Error(log, line.c_str()); + } + + free(symbols); +} + +/* Note that this signal handler is not async signal safe ! + * Ideally a signal handler should only flip a bit and defer + * heavy work to some kind of bottom-half processing. */ +void signal_handler(int signum, siginfo_t * /*info*/, void * /*context*/) +{ + Logger* log = Logger::getInstance("SIGNAL"); + + char *name = strsignal(signum); + if (name) + { + Info(log, "Signal received : %s", name); + } + + switch(signum) + { + case SIGSEGV: + case SIGABRT: + case SIGFPE : + print_trace(); + exit(1); + case SIGINT : + case SIGTERM: + case SIGPIPE: + default: + /* If the signal_handler is hit before the event loop is started, + * following call will do nothing. So we queue the call. */ + + // QCoreApplication::quit(); + QMetaObject::invokeMethod(qApp, "quit", Qt::QueuedConnection); + + // Reset signal handler to default (in case this handler is not capable of stopping) + struct sigaction action{}; + action.sa_handler = SIG_DFL; + sigaction(signum, &action, nullptr); + + } +} + +} // namespace DefaultSignalHandler +#endif // _WIN32 + +namespace DefaultSignalHandler +{ +void install() +{ +#ifndef _WIN32 + struct sigaction action{}; + action.sa_sigaction = signal_handler; + action.sa_flags = SA_RESTART | SA_SIGINFO; + + sigaction(SIGHUP , &action, nullptr); + sigaction(SIGFPE , &action, nullptr); + sigaction(SIGINT , &action, nullptr); + sigaction(SIGTERM, &action, nullptr); + sigaction(SIGABRT, &action, nullptr); + sigaction(SIGSEGV, &action, nullptr); + sigaction(SIGPIPE, &action, nullptr); +#endif // _WIN32 +} +} // namespace DefaultSignalHandler diff --git a/src/hyperion-aml/hyperion-aml.cpp b/src/hyperion-aml/hyperion-aml.cpp index 9256181a..2dfed16f 100644 --- a/src/hyperion-aml/hyperion-aml.cpp +++ b/src/hyperion-aml/hyperion-aml.cpp @@ -12,6 +12,8 @@ // ssdp discover #include +#include + using namespace commandline; // save the image as screenshot @@ -29,6 +31,8 @@ int main(int argc, char ** argv) << "\tVersion : " << HYPERION_VERSION << " (" << HYPERION_BUILD_ID << ")" << std::endl << "\tbuild time: " << __DATE__ << " " << __TIME__ << std::endl; + DefaultSignalHandler::install(); + QCoreApplication app(argc, argv); try diff --git a/src/hyperion-dispmanx/hyperion-dispmanx.cpp b/src/hyperion-dispmanx/hyperion-dispmanx.cpp index 77b8dbc5..1e784112 100644 --- a/src/hyperion-dispmanx/hyperion-dispmanx.cpp +++ b/src/hyperion-dispmanx/hyperion-dispmanx.cpp @@ -12,6 +12,8 @@ // ssdp discover #include +#include + using namespace commandline; // save the image as screenshot @@ -29,6 +31,8 @@ int main(int argc, char ** argv) << "\tVersion : " << HYPERION_VERSION << " (" << HYPERION_BUILD_ID << ")" << std::endl << "\tbuild time: " << __DATE__ << " " << __TIME__ << std::endl; + DefaultSignalHandler::install(); + QCoreApplication app(argc, argv); try diff --git a/src/hyperion-framebuffer/hyperion-framebuffer.cpp b/src/hyperion-framebuffer/hyperion-framebuffer.cpp index 02833bb2..35847cb5 100644 --- a/src/hyperion-framebuffer/hyperion-framebuffer.cpp +++ b/src/hyperion-framebuffer/hyperion-framebuffer.cpp @@ -9,6 +9,8 @@ // ssdp discover #include +#include + using namespace commandline; // save the image as screenshot @@ -21,6 +23,8 @@ void saveScreenshot(QString filename, const Image & image) int main(int argc, char ** argv) { + DefaultSignalHandler::install(); + QCoreApplication app(argc, argv); try diff --git a/src/hyperion-osx/hyperion-osx.cpp b/src/hyperion-osx/hyperion-osx.cpp index 09c2cc0d..4842c65d 100644 --- a/src/hyperion-osx/hyperion-osx.cpp +++ b/src/hyperion-osx/hyperion-osx.cpp @@ -11,6 +11,8 @@ // ssdp discover #include +#include + using namespace commandline; // save the image as screenshot @@ -23,6 +25,8 @@ void saveScreenshot(QString filename, const Image & image) int main(int argc, char ** argv) { + DefaultSignalHandler::install(); + QCoreApplication app(argc, argv); try diff --git a/src/hyperion-remote/hyperion-remote.cpp b/src/hyperion-remote/hyperion-remote.cpp index 32714bd2..a1370f32 100644 --- a/src/hyperion-remote/hyperion-remote.cpp +++ b/src/hyperion-remote/hyperion-remote.cpp @@ -17,6 +17,7 @@ #include "HyperionConfig.h" #include +#include using namespace commandline; @@ -66,6 +67,8 @@ int main(int argc, char * argv[]) << "\tVersion : " << HYPERION_VERSION << " (" << HYPERION_BUILD_ID << ")" << std::endl << "\tbuild time: " << __DATE__ << " " << __TIME__ << std::endl; + DefaultSignalHandler::install(); + QCoreApplication app(argc, argv); // force the locale diff --git a/src/hyperion-v4l2/hyperion-v4l2.cpp b/src/hyperion-v4l2/hyperion-v4l2.cpp index 2c8ba3c3..6c7c6dc2 100644 --- a/src/hyperion-v4l2/hyperion-v4l2.cpp +++ b/src/hyperion-v4l2/hyperion-v4l2.cpp @@ -24,6 +24,8 @@ // ssdp discover #include +#include + using namespace commandline; int main(int argc, char** argv) diff --git a/src/hyperion-x11/hyperion-x11.cpp b/src/hyperion-x11/hyperion-x11.cpp index 08a624a0..9d193d12 100644 --- a/src/hyperion-x11/hyperion-x11.cpp +++ b/src/hyperion-x11/hyperion-x11.cpp @@ -5,6 +5,7 @@ #include #include +#include #include "X11Wrapper.h" #include "HyperionConfig.h" @@ -28,6 +29,8 @@ int main(int argc, char ** argv) << "\tVersion : " << HYPERION_VERSION << " (" << HYPERION_BUILD_ID << ")" << std::endl << "\tbuild time: " << __DATE__ << " " << __TIME__ << std::endl; + DefaultSignalHandler::install(); + QApplication app(argc, argv); try { diff --git a/src/hyperiond/detectProcess.h b/src/hyperiond/detectProcess.h index 48476a61..8d5b622f 100644 --- a/src/hyperiond/detectProcess.h +++ b/src/hyperiond/detectProcess.h @@ -11,54 +11,42 @@ // psapi.h requires windows.h to be included #include #include +#include #endif -unsigned int getProcessIdsByProcessName(const char *processName, QStringList &listOfPids) +QStringList getProcessIdsByProcessName(const char *processName) { - // Clear content of returned list of PIDS - listOfPids.clear(); + QStringList listOfPids; #if defined(WIN32) - // Get the list of process identifiers. - DWORD aProcesses[1024], cbNeeded, cProcesses; - unsigned int i; - - if (!EnumProcesses(aProcesses, sizeof(aProcesses), &cbNeeded)) - return 0; - - // Calculate how many process identifiers were returned. - cProcesses = cbNeeded / sizeof(DWORD); - - // Search for a matching name for each process - for (i = 0; i < cProcesses; i++) + // https://docs.microsoft.com/en-us/windows/win32/toolhelp/taking-a-snapshot-and-viewing-processes + /* Take a snapshot of all processes in the system */ + HANDLE hProcessSnap = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0); + if(hProcessSnap == INVALID_HANDLE_VALUE) { - if (aProcesses[i] != 0) - { - char szProcessName[MAX_PATH] = {0}; - - DWORD processID = aProcesses[i]; - - // Get a handle to the process. - HANDLE hProcess = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, processID); - - // Get the process name - if (NULL != hProcess) - { - HMODULE hMod; - DWORD cbNeeded; - - if (EnumProcessModules(hProcess, &hMod, sizeof(hMod), &cbNeeded)) - GetModuleBaseNameA(hProcess, hMod, szProcessName, sizeof(szProcessName) / sizeof(char)); - - // Release the handle to the process. - CloseHandle(hProcess); - - if (*szProcessName != 0 && strcmp(processName, szProcessName) == 0) - listOfPids.append(QString::number(processID)); - } - } + return {}; } + PROCESSENTRY32 pe32{}; + pe32.dwSize = sizeof(PROCESSENTRY32); + + /* Retrieve information about the first process */ + if(!Process32First(hProcessSnap, &pe32)) + { + CloseHandle(hProcessSnap); + return {}; + } + + /* Walk through the snapshot of processes */ + do + { + if (strcmp(processName, pe32.szExeFile) == 0) + listOfPids.append(QString::number(pe32.th32ProcessID)); + + } while(Process32Next(hProcessSnap, &pe32)); + + CloseHandle(hProcessSnap); + #else QDir dir("/proc"); @@ -90,5 +78,5 @@ unsigned int getProcessIdsByProcessName(const char *processName, QStringList &li #endif - return listOfPids.count(); + return listOfPids; } diff --git a/src/hyperiond/main.cpp b/src/hyperiond/main.cpp index 94d0f991..494e5113 100644 --- a/src/hyperiond/main.cpp +++ b/src/hyperiond/main.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include <../../include/db/AuthTable.h> #include "detectProcess.h" @@ -76,11 +77,6 @@ void signal_handler(const int signum) } return; } - - QCoreApplication::quit(); - - // reset signal handler to default (in case this handler is not capable of stopping) - signal(signum, SIG_DFL); } #endif @@ -150,14 +146,13 @@ int main(int argc, char** argv) // check if we are running already an instance // TODO Allow one session per user - // http://www.qtcentre.org/threads/44489-Get-Process-ID-for-a-running-application - QStringList listOfPids; #ifdef _WIN32 const char* processName = "hyperiond.exe"; #else const char* processName = "hyperiond"; #endif - if (getProcessIdsByProcessName(processName, listOfPids) > 1) + const QStringList listOfPids = getProcessIdsByProcessName(processName); + if (listOfPids.size() > 1) { Error(log, "The Hyperion Daemon is already running, abort start"); return 0; @@ -168,12 +163,10 @@ int main(int argc, char** argv) bool isGuiApp = (qobject_cast(app.data()) != 0 && QSystemTrayIcon::isSystemTrayAvailable()); + DefaultSignalHandler::install(); + #ifndef _WIN32 - signal(SIGINT, signal_handler); - signal(SIGTERM, signal_handler); - signal(SIGABRT, signal_handler); signal(SIGCHLD, signal_handler); - signal(SIGPIPE, signal_handler); signal(SIGUSR1, signal_handler); signal(SIGUSR2, signal_handler); #endif @@ -309,7 +302,7 @@ int main(int argc, char** argv) // delete database before start if(parser.isSet(deleteDB)) { - QString dbFile = mDir.absolutePath() + "/db/hyperion.db"; + const QString dbFile = mDir.absolutePath() + "/db/hyperion.db"; if (QFile::exists(dbFile)) { if (!QFile::remove(dbFile)) diff --git a/src/hyperiond/systray.cpp b/src/hyperiond/systray.cpp index bd962156..0ed989a7 100644 --- a/src/hyperiond/systray.cpp +++ b/src/hyperiond/systray.cpp @@ -127,7 +127,7 @@ void SysTray::closeEvent(QCloseEvent *event) void SysTray::settings() { -#ifndef _WIN32 + #ifndef _WIN32 // Hide error messages when opening webbrowser int out_pipe[2];