From 548968279926a73d7ff19a9a185c977c50d56756 Mon Sep 17 00:00:00 2001 From: Tom Englund Date: Wed, 31 Jul 2024 21:00:14 +0200 Subject: [PATCH] internal: some minor fd/socket cleanups and make logging thread safe (#7123) * bezier: dont loop on float values Using a floating-point loop variable with a fixed increment can cause precision errors over time due to the nature of floating-point arithmetic. and cause undesired effects. ex iteration 1 = 0.10000000149011611938 iteration 2 = 0.20000000298023223877 eventually.. iteration 8 = 0.80000001192092895508 iteration 9 = 0.89999997615814208984 * hyprctl: close sockets on destruction store socketpath and close the fd and unlink the socket path on exit. * eventloopmgr: close the timerfd close the timerfd on exit. * debug: make logging thread safe instead of opening and closing the logfile on each write open it on init and close it on compositor exit. also add a mutex so accidently using logging from a thread like the watchdog or similiar doesnt cause issues. * xwl: clean up fd logic check if the fd is actually opened before closing, and close the pipesource FD on exit. --- src/Compositor.cpp | 2 ++ src/debug/HyprCtl.cpp | 10 +++++++--- src/debug/HyprCtl.hpp | 1 + src/debug/Log.cpp | 12 +++++++----- src/debug/Log.hpp | 7 +++++++ src/helpers/BezierCurve.cpp | 4 +++- src/managers/eventLoop/EventLoopManager.cpp | 2 ++ src/xwayland/Server.cpp | 12 ++++++++---- src/xwayland/Server.hpp | 1 + 9 files changed, 38 insertions(+), 13 deletions(-) diff --git a/src/Compositor.cpp b/src/Compositor.cpp index 97355d2c..7221d3a7 100644 --- a/src/Compositor.cpp +++ b/src/Compositor.cpp @@ -518,6 +518,8 @@ void CCompositor::cleanup() { // this frees all wayland resources, including sockets wl_display_destroy(m_sWLDisplay); + + Debug::close(); } void CCompositor::initManagers(eManagersInitStage stage) { diff --git a/src/debug/HyprCtl.cpp b/src/debug/HyprCtl.cpp index cf873e6c..d91a1cec 100644 --- a/src/debug/HyprCtl.cpp +++ b/src/debug/HyprCtl.cpp @@ -1604,6 +1604,10 @@ CHyprCtl::CHyprCtl() { CHyprCtl::~CHyprCtl() { if (m_eventSource) wl_event_source_remove(m_eventSource); + if (m_iSocketFD >= 0) + close(m_iSocketFD); + if (!m_socketPath.empty()) + unlink(m_socketPath.c_str()); } SP CHyprCtl::registerCommand(SHyprCtlCommand cmd) { @@ -1821,9 +1825,9 @@ void CHyprCtl::startHyprCtlSocket() { sockaddr_un SERVERADDRESS = {.sun_family = AF_UNIX}; - std::string socketPath = g_pCompositor->m_szInstancePath + "/.socket.sock"; + m_socketPath = g_pCompositor->m_szInstancePath + "/.socket.sock"; - strcpy(SERVERADDRESS.sun_path, socketPath.c_str()); + strcpy(SERVERADDRESS.sun_path, m_socketPath.c_str()); if (bind(m_iSocketFD, (sockaddr*)&SERVERADDRESS, SUN_LEN(&SERVERADDRESS)) < 0) { Debug::log(ERR, "Couldn't start the Hyprland Socket. (2) IPC will not work."); @@ -1833,7 +1837,7 @@ void CHyprCtl::startHyprCtlSocket() { // 10 max queued. listen(m_iSocketFD, 10); - Debug::log(LOG, "Hypr socket started at {}", socketPath); + Debug::log(LOG, "Hypr socket started at {}", m_socketPath); m_eventSource = wl_event_loop_add_fd(g_pCompositor->m_sWLEventLoop, m_iSocketFD, WL_EVENT_READABLE, hyprCtlFDTick, nullptr); } diff --git a/src/debug/HyprCtl.hpp b/src/debug/HyprCtl.hpp index ccbd40cd..cbacd7cb 100644 --- a/src/debug/HyprCtl.hpp +++ b/src/debug/HyprCtl.hpp @@ -31,6 +31,7 @@ class CHyprCtl { std::vector> m_vCommands; wl_event_source* m_eventSource = nullptr; + std::string m_socketPath; }; inline std::unique_ptr g_pHyprCtl; diff --git a/src/debug/Log.cpp b/src/debug/Log.cpp index c2939831..0def77c0 100644 --- a/src/debug/Log.cpp +++ b/src/debug/Log.cpp @@ -8,6 +8,11 @@ void Debug::init(const std::string& IS) { logFile = IS + (ISDEBUG ? "/hyprlandd.log" : "/hyprland.log"); + logOfs.open(logFile, std::ios::out | std::ios::app); +} + +void Debug::close() { + logOfs.close(); } void Debug::log(LogLevel level, std::string str) { @@ -55,11 +60,8 @@ void Debug::log(LogLevel level, std::string str) { if (!disableLogs || !**disableLogs) { // log to a file - std::ofstream ofs; - ofs.open(logFile, std::ios::out | std::ios::app); - ofs << str << "\n"; - - ofs.close(); + logOfs << str << "\n"; + logOfs.flush(); } // log it to the stdout too. diff --git a/src/debug/Log.hpp b/src/debug/Log.hpp index 33f3c9c1..617f451a 100644 --- a/src/debug/Log.hpp +++ b/src/debug/Log.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #include "../includes.hpp" #include "../helpers/MiscFunctions.hpp" @@ -22,6 +23,7 @@ enum LogLevel { namespace Debug { inline std::string logFile; + inline std::ofstream logOfs; inline int64_t* const* disableLogs = nullptr; inline int64_t* const* disableTime = nullptr; inline bool disableStdout = false; @@ -30,14 +32,18 @@ namespace Debug { inline int64_t* const* coloredLogs = nullptr; inline std::string rollingLog = ""; // rolling log contains the ROLLING_LOG_SIZE tail of the log + inline std::mutex logMutex; void init(const std::string& IS); + void close(); // void log(LogLevel level, std::string str); template void log(LogLevel level, std::format_string fmt, Args&&... args) { + std::lock_guard guard(logMutex); + if (level == TRACE && !trace) return; @@ -66,5 +72,6 @@ namespace Debug { logMsg += std::vformat(fmt.get(), std::make_format_args(args...)); log(level, logMsg); + logMutex.unlock(); } }; diff --git a/src/helpers/BezierCurve.cpp b/src/helpers/BezierCurve.cpp index e79863a3..dd0ff2b0 100644 --- a/src/helpers/BezierCurve.cpp +++ b/src/helpers/BezierCurve.cpp @@ -30,8 +30,10 @@ void CBezierCurve::setup(std::vector* pVec) { const auto POINTSSIZE = m_aPointsBaked.size() * sizeof(m_aPointsBaked[0]) / 1000.f; const auto BEGINCALC = std::chrono::high_resolution_clock::now(); - for (float i = 0.1f; i < 1.f; i += 0.1f) + for (int j = 1; j < 10; ++j) { + float i = j / 10.0f; getYForPoint(i); + } const auto ELAPSEDCALCAVG = std::chrono::duration_cast(std::chrono::high_resolution_clock::now() - BEGINCALC).count() / 1000.f / 10.f; Debug::log(LOG, "Created a bezier curve, baked {} points, mem usage: {:.2f}kB, time to bake: {:.2f}µs. Estimated average calc time: {:.2f}µs.", BAKEDPOINTS, POINTSSIZE, diff --git a/src/managers/eventLoop/EventLoopManager.cpp b/src/managers/eventLoop/EventLoopManager.cpp index 3131531a..c2c088f8 100644 --- a/src/managers/eventLoop/EventLoopManager.cpp +++ b/src/managers/eventLoop/EventLoopManager.cpp @@ -27,6 +27,8 @@ CEventLoopManager::~CEventLoopManager() { wl_event_source_remove(m_sWayland.eventSource); if (m_sIdle.eventSource) wl_event_source_remove(m_sIdle.eventSource); + if (m_sTimers.timerfd >= 0) + close(m_sTimers.timerfd); } static int timerWrite(int fd, uint32_t mask, void* data) { diff --git a/src/xwayland/Server.cpp b/src/xwayland/Server.cpp index 3f4e7b43..cec582f6 100644 --- a/src/xwayland/Server.cpp +++ b/src/xwayland/Server.cpp @@ -262,13 +262,16 @@ void CXWaylandServer::die() { if (pipeSource) wl_event_source_remove(pipeSource); - if (waylandFDs[0]) + if (pipeFd >= 0) + close(pipeFd); + + if (waylandFDs[0] >= 0) close(waylandFDs[0]); - if (waylandFDs[1]) + if (waylandFDs[1] >= 0) close(waylandFDs[1]); - if (xwmFDs[0]) + if (xwmFDs[0] >= 0) close(xwmFDs[0]); - if (xwmFDs[1]) + if (xwmFDs[1] >= 0) close(xwmFDs[1]); // possible crash. Better to leak a bit. @@ -364,6 +367,7 @@ bool CXWaylandServer::start() { } pipeSource = wl_event_loop_add_fd(g_pCompositor->m_sWLEventLoop, notify[0], WL_EVENT_READABLE, ::xwaylandReady, nullptr); + pipeFd = notify[0]; serverPID = fork(); if (serverPID < 0) { diff --git a/src/xwayland/Server.hpp b/src/xwayland/Server.hpp index 0c06a56c..7a36a965 100644 --- a/src/xwayland/Server.hpp +++ b/src/xwayland/Server.hpp @@ -41,6 +41,7 @@ class CXWaylandServer { std::array xFDReadEvents = {nullptr, nullptr}; wl_event_source* idleSource = nullptr; wl_event_source* pipeSource = nullptr; + int pipeFd = -1; std::array xwmFDs = {-1, -1}; std::array waylandFDs = {-1, -1};