From cedda6fbddb4bc1321d33e0a63ff8f385f29945f Mon Sep 17 00:00:00 2001 From: Geoffrey Merck Date: Sat, 23 Aug 2025 14:25:48 +0200 Subject: [PATCH 1/5] add some more tests for logRepeat #58 --- Tests/Log/logRepeat.cpp | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/Tests/Log/logRepeat.cpp b/Tests/Log/logRepeat.cpp index 99a6b8a..d39cbe5 100644 --- a/Tests/Log/logRepeat.cpp +++ b/Tests/Log/logRepeat.cpp @@ -52,6 +52,18 @@ namespace LogRepeatTests EXPECT_THAT(m_logTarget->m_messages[1].c_str(), EndsWith("[ERROR ] Two Message\n")); } + TEST_F(LogRepeat, ThreeIdenticalMessageThreshold0) { + CLog::getRepeatThreshold() = 0U; + CLog::logError("One Message"); + CLog::logError("One Message"); + CLog::logError("One Message"); + + EXPECT_EQ(3, m_logTarget->m_messages.size()) << "There should be 3 messages in the log."; + EXPECT_THAT(m_logTarget->m_messages[0].c_str(), EndsWith("[ERROR ] One Message\n")); + EXPECT_THAT(m_logTarget->m_messages[1].c_str(), EndsWith("[ERROR ] One Message\n")); + EXPECT_THAT(m_logTarget->m_messages[2].c_str(), EndsWith("[ERROR ] One Message\n")); + } + TEST_F(LogRepeat, ThreeIdenticalMessageThreshold1) { CLog::getRepeatThreshold() = 1U; CLog::logError("One Message"); @@ -62,6 +74,34 @@ namespace LogRepeatTests EXPECT_THAT(m_logTarget->m_messages[0].c_str(), EndsWith("[ERROR ] One Message\n")); } + TEST_F(LogRepeat, NineIdenticalMessageTwoDifferentThreshold0) { + CLog::getRepeatThreshold() = 0U; + CLog::logError("One Message"); + CLog::logError("One Message"); + CLog::logError("One Message"); + CLog::logError("One Message"); + CLog::logError("One Message"); + CLog::logError("One Message"); + CLog::logError("One Message"); + CLog::logError("One Message"); + CLog::logError("One Message"); + CLog::logError("Another Message"); + CLog::logError("And here is another Message"); + + EXPECT_EQ(11, m_logTarget->m_messages.size()) << "There should be two message in the log."; + EXPECT_THAT(m_logTarget->m_messages[0].c_str(), EndsWith("[ERROR ] One Message\n")); + EXPECT_THAT(m_logTarget->m_messages[1].c_str(), EndsWith("[ERROR ] One Message\n")); + EXPECT_THAT(m_logTarget->m_messages[2].c_str(), EndsWith("[ERROR ] One Message\n")); + EXPECT_THAT(m_logTarget->m_messages[3].c_str(), EndsWith("[ERROR ] One Message\n")); + EXPECT_THAT(m_logTarget->m_messages[4].c_str(), EndsWith("[ERROR ] One Message\n")); + EXPECT_THAT(m_logTarget->m_messages[5].c_str(), EndsWith("[ERROR ] One Message\n")); + EXPECT_THAT(m_logTarget->m_messages[6].c_str(), EndsWith("[ERROR ] One Message\n")); + EXPECT_THAT(m_logTarget->m_messages[7].c_str(), EndsWith("[ERROR ] One Message\n")); + EXPECT_THAT(m_logTarget->m_messages[8].c_str(), EndsWith("[ERROR ] One Message\n")); + EXPECT_THAT(m_logTarget->m_messages[9].c_str(), EndsWith("[ERROR ] Another Message\n")); + EXPECT_THAT(m_logTarget->m_messages[10].c_str(), EndsWith("[ERROR ] And here is another Message\n")); + } + TEST_F(LogRepeat, NineIdenticalMessageTwoDifferentThreshold1) { CLog::getRepeatThreshold() = 1U; CLog::logError("One Message"); From 64636ea729f46e1b87ab4c0a776ba1e235fdc041 Mon Sep 17 00:00:00 2001 From: Geoffrey Merck Date: Sat, 23 Aug 2025 14:28:12 +0200 Subject: [PATCH 2/5] fix log repeat threshold not set from config #58 --- DStarGateway/DStarGatewayApp.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/DStarGateway/DStarGatewayApp.cpp b/DStarGateway/DStarGatewayApp.cpp index f222a0c..cf01aee 100644 --- a/DStarGateway/DStarGatewayApp.cpp +++ b/DStarGateway/DStarGatewayApp.cpp @@ -118,6 +118,7 @@ int main(int argc, char *argv[]) TLog logConf; config->getLog(logConf); CLog::finalise(); + CLog::getRepeatThreshold() = logConf.repeatThreshold; if(logConf.displayLevel != LOG_NONE && !daemon.daemon) CLog::addTarget(new CLogConsoleTarget(logConf.displayLevel)); if(logConf.fileLevel != LOG_NONE) CLog::addTarget(new CLogFileTarget(logConf.fileLevel, logConf.logDir, logConf.fileRoot, logConf.fileRotate)); @@ -162,6 +163,7 @@ bool CDStarGatewayApp::init() void CDStarGatewayApp::run() { + CLog::logInfo("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"); m_thread->Run(); m_thread->Wait(); CLog::logInfo("exiting\n"); From 934ab80f95cf23dc30b10582ac4b6cf8372bff37 Mon Sep 17 00:00:00 2001 From: Geoffrey Merck Date: Sat, 23 Aug 2025 14:46:48 +0200 Subject: [PATCH 3/5] no more recursivity in log #58 --- BaseCommon/Log.h | 104 ++++++++++++++++++++++++++++++----------------- 1 file changed, 66 insertions(+), 38 deletions(-) diff --git a/BaseCommon/Log.h b/BaseCommon/Log.h index 7bf9638..8b43903 100644 --- a/BaseCommon/Log.h +++ b/BaseCommon/Log.h @@ -16,7 +16,6 @@ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ - #pragma once #include @@ -39,13 +38,13 @@ private: static uint m_prevMsgCount; static uint m_repeatThreshold; - static void getTimeStamp(std::string& s); + static void getTimeStamp(std::string &s); - template - static void formatLogMessage(std::string& output, LOG_SEVERITY severity, const std::string & f, Args... args) + template + static void formatLogMessage(std::string &output, LOG_SEVERITY severity, const std::string &f, Args... args) { assert(severity != LOG_NONE); - + std::string severityStr(" "); switch (severity) { @@ -58,7 +57,7 @@ private: case LOG_FATAL: severityStr.assign("FATAL "); break; - case LOG_INFO : + case LOG_INFO: severityStr.assign("INFO "); break; case LOG_WARNING: @@ -74,84 +73,113 @@ private: std::string f2("[%s] "); f2.append(f); CStringUtils::string_format_in_place(output, f2, severityStr.c_str(), args...); - boost::trim_if(output, [](char c){ return c == '\n' || c == '\r' || c == ' ' || c == '\t'; }); + boost::trim_if(output, [](char c) + { return c == '\n' || c == '\r' || c == ' ' || c == '\t'; }); output.push_back('\n'); } public: - static void addTarget(CLogTarget * target); + static void addTarget(CLogTarget *target); static void finalise(); - static uint& getRepeatThreshold(); + static uint &getRepeatThreshold(); - template static void logTrace(const std::string & f, Args... args) + template + static void logTrace(const std::string &f, Args... args) { log(LOG_TRACE, f, args...); } - template static void logDebug(const std::string & f, Args... args) + template + static void logDebug(const std::string &f, Args... args) { log(LOG_DEBUG, f, args...); } - template static void logInfo(const std::string & f, Args... args) + template + static void logInfo(const std::string &f, Args... args) { log(LOG_INFO, f, args...); } - template static void logWarning(const std::string & f, Args... args) + template + static void logWarning(const std::string &f, Args... args) { log(LOG_WARNING, f, args...); } - template static void logError(const std::string & f, Args... args) + template + static void logError(const std::string &f, Args... args) { log(LOG_ERROR, f, args...); } - template static void logFatal(const std::string & f, Args... args) + template + static void logFatal(const std::string &f, Args... args) { log(LOG_FATAL, f, args...); } - template static void log(LOG_SEVERITY severity, const std::string & f, Args... args) + template + static void log(LOG_SEVERITY severity, const std::string &f, Args... args) { + // Protect against concurrent access to log targets std::lock_guard lockTarget(m_targetsMutex); - if(m_targets.empty()) + if (m_targets.empty()) return; + // Format the message with the given arguments std::string msg; formatLogMessage(msg, severity, f, args...); - bool repeatedMsg = (msg.compare(m_prevMsg) == 0); + bool repeatedMsg = (msg == m_prevMsg); - if(repeatedMsg && m_repeatThreshold > 0U) { + // Handle repeated messages + if (repeatedMsg && m_repeatThreshold > 0U) { m_prevMsgCount++; - if(m_prevMsgCount >= m_repeatThreshold) + if (m_prevMsgCount >= m_repeatThreshold) + { + // If threshold reached, skip logging this duplicate return; + } } - - m_prevMsg.assign(msg); - if(m_prevMsgCount >= m_repeatThreshold && !repeatedMsg && m_repeatThreshold > 0U) { - formatLogMessage(msg, severity, "Previous message repeated %d times", m_prevMsgCount - m_repeatThreshold + 1); - m_prevMsg.clear(); - } - - std::string timestamp; - getTimeStamp(timestamp); - std::string msgts; - CStringUtils::string_format_in_place(msgts, "[%s] %s", timestamp.c_str(), msg.c_str()); - - for(auto target : m_targets) { - if(severity >= target->getLevel()) { - target->printLog(msgts); + // If we are leaving a repetition sequence, log a summary first + if (!repeatedMsg && m_repeatThreshold > 0U && m_prevMsgCount >= m_repeatThreshold) { + std::string summary; + formatLogMessage(summary, severity, + "Previous message repeated %d times", + m_prevMsgCount - m_repeatThreshold + 1); + + std::string ts; + getTimeStamp(ts); + + std::string summaryLine; + CStringUtils::string_format_in_place(summaryLine, "[%s] %s", ts.c_str(), summary.c_str()); + + for (auto target : m_targets) + { + if (severity >= target->getLevel()) + target->printLog(summaryLine); } - } - if(m_prevMsgCount != 0 && !repeatedMsg) { + // Reset repetition counter after summary m_prevMsgCount = 0; - log(severity, f, args ...); } + + // Always log the current message + std::string ts; + getTimeStamp(ts); + + std::string msgLine; + CStringUtils::string_format_in_place(msgLine, "[%s] %s", ts.c_str(), msg.c_str()); + + for (auto target : m_targets) { + if (severity >= target->getLevel()) + target->printLog(msgLine); + } + + // Save current message for repetition detection + m_prevMsg = msg; } }; From ee36452b010f399786514ed39adfdae5b103599e Mon Sep 17 00:00:00 2001 From: Geoffrey Merck Date: Sat, 23 Aug 2025 14:50:55 +0200 Subject: [PATCH 4/5] remove test logging message #58 --- DStarGateway/DStarGatewayApp.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/DStarGateway/DStarGatewayApp.cpp b/DStarGateway/DStarGatewayApp.cpp index cf01aee..c2f0c7b 100644 --- a/DStarGateway/DStarGatewayApp.cpp +++ b/DStarGateway/DStarGatewayApp.cpp @@ -163,7 +163,6 @@ bool CDStarGatewayApp::init() void CDStarGatewayApp::run() { - CLog::logInfo("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"); m_thread->Run(); m_thread->Wait(); CLog::logInfo("exiting\n"); From cf8e19684da76cbf26c33aac9b1294873e0e479e Mon Sep 17 00:00:00 2001 From: Geoffrey Merck Date: Sat, 23 Aug 2025 14:55:06 +0200 Subject: [PATCH 5/5] change from future to actual thread #58 --- IRCDDB/IRCClient.cpp | 21 +++++++++++++-------- IRCDDB/IRCClient.h | 9 +++++---- IRCDDB/IRCDDBApp.cpp | 22 ++++++++++++++-------- IRCDDB/IRCDDBApp.h | 6 ++++-- 4 files changed, 36 insertions(+), 22 deletions(-) diff --git a/IRCDDB/IRCClient.cpp b/IRCDDB/IRCClient.cpp index dcdc3cf..d46b32b 100644 --- a/IRCDDB/IRCClient.cpp +++ b/IRCDDB/IRCClient.cpp @@ -58,19 +58,24 @@ IRCClient::IRCClient(IRCApplication *app, const std::string& update_channel, con IRCClient::~IRCClient() { - delete m_proto; + stopWork(); + delete m_proto; } void IRCClient::startWork() { - m_terminateThread = false; - m_future = std::async(std::launch::async, &IRCClient::Entry, this); + if (m_thread.joinable()) + return; + + m_terminateThread.store(false, std::memory_order_relaxed); + m_thread = std::thread(&IRCClient::Entry, this); } void IRCClient::stopWork() { - m_terminateThread = true; - m_future.get(); + m_terminateThread.store(true, std::memory_order_relaxed); + if (m_thread.joinable()) + m_thread.join(); } void IRCClient::Entry() @@ -98,7 +103,7 @@ void IRCClient::Entry() switch (state) { case 0: - if (m_terminateThread) { + if (m_terminateThread.load(std::memory_order_relaxed)) { CLog::logInfo("IRCClient::Entry: thread terminated at state=%d\n", state); return; } @@ -118,7 +123,7 @@ void IRCClient::Entry() break; case 1: - if (m_terminateThread) { + if (m_terminateThread.load(std::memory_order_relaxed)) { CLog::logInfo("IRCClient::Entry: thread terminated at state=%d\n", state); return; } @@ -261,7 +266,7 @@ void IRCClient::Entry() case 5: - if (m_terminateThread) + if (m_terminateThread.load(std::memory_order_relaxed)) state = 6; else { if (m_recvQ->isEOF()) { diff --git a/IRCDDB/IRCClient.h b/IRCDDB/IRCClient.h index 72b6c97..2a1a023 100644 --- a/IRCDDB/IRCClient.h +++ b/IRCDDB/IRCClient.h @@ -22,7 +22,8 @@ along with this program. If not, see . #pragma once #include -#include +#include +#include #include "IRCReceiver.h" #include "IRCMessageQueue.h" @@ -47,13 +48,13 @@ private: unsigned int m_port; std::string m_callsign; std::string m_password; - bool m_terminateThread; + std::atomic m_terminateThread{false}; IRCReceiver *m_recv; IRCMessageQueue *m_recvQ; IRCMessageQueue *m_sendQ; IRCProtocol *m_proto; IRCApplication *m_app; - std::future m_future; -}; + std::thread m_thread; +}; \ No newline at end of file diff --git a/IRCDDB/IRCDDBApp.cpp b/IRCDDB/IRCDDBApp.cpp index 58909de..862ed24 100644 --- a/IRCDDB/IRCDDBApp.cpp +++ b/IRCDDB/IRCDDBApp.cpp @@ -112,7 +112,7 @@ public: std::regex m_fromPattern; bool m_initReady; - bool m_terminateThread; + std::atomic m_terminateThread{false}; std::map m_userMap; std::mutex m_userMapMutex; @@ -151,8 +151,10 @@ IRCDDBApp::IRCDDBApp(const std::string& u_chan) IRCDDBApp::~IRCDDBApp() { - delete m_d->m_sendQ; - delete m_d; + stopWork(); + + delete m_d->m_sendQ; + delete m_d; } void IRCDDBApp::rptrQTH(const std::string& callsign, double latitude, double longitude, const std::string& desc1, const std::string& desc2, const std::string& infoURL) @@ -279,14 +281,18 @@ IRCMessage *IRCDDBApp::getReplyMessage() void IRCDDBApp::startWork() { - m_d->m_terminateThread = false; - m_future = std::async(std::launch::async, &IRCDDBApp::Entry, this); + if (m_thread.joinable()) + return; + + m_d->m_terminateThread.store(false, std::memory_order_relaxed); + m_thread = std::thread(&IRCDDBApp::Entry, this); } void IRCDDBApp::stopWork() { - m_d->m_terminateThread = true; - m_future.get(); + m_d->m_terminateThread.store(true, std::memory_order_relaxed); + if (m_thread.joinable()) + m_thread.join(); } unsigned int IRCDDBApp::calculateUsn(const std::string& nick) @@ -996,7 +1002,7 @@ static bool needsDatabaseUpdate(int tableID) void IRCDDBApp::Entry() { int sendlistTableID = 0; - while (!m_d->m_terminateThread) { + while (!m_d->m_terminateThread.load(std::memory_order_relaxed)) { if (m_d->m_timer > 0) m_d->m_timer--; switch(m_d->m_state) { diff --git a/IRCDDB/IRCDDBApp.h b/IRCDDB/IRCDDBApp.h index a53ed34..85f6251 100644 --- a/IRCDDB/IRCDDBApp.h +++ b/IRCDDB/IRCDDBApp.h @@ -25,7 +25,8 @@ along with this program. If not, see . #include "IRCApplication.h" #include -#include +#include +#include #include #include @@ -99,6 +100,7 @@ private: IRCDDBAppPrivate *m_d; time_t m_maxTime; - std::future m_future; + std::thread m_thread; + };