From 66ca3e5b4c2969f98a45404bd9aff13213718c98 Mon Sep 17 00:00:00 2001 From: Wu Shangwei <2826256824@qq.com> Date: Wed, 7 Jul 2021 00:59:57 +0800 Subject: [PATCH 1/3] Seperate TYPE_CONTROL from TYPE_QUERIER Signed-off-by: Wu Shangwei <2826256824@qq.com> --- services/hilogd/include/log_persister.h | 2 +- services/hilogd/include/log_querier.h | 2 +- services/hilogd/include/log_reader.h | 3 ++- services/hilogd/log_collector.cpp | 4 +++- services/hilogd/log_persister.cpp | 2 +- services/hilogd/log_querier.cpp | 11 +++++++++-- 6 files changed, 17 insertions(+), 7 deletions(-) diff --git a/services/hilogd/include/log_persister.h b/services/hilogd/include/log_persister.h index 74dd580..f2a15ba 100644 --- a/services/hilogd/include/log_persister.h +++ b/services/hilogd/include/log_persister.h @@ -56,7 +56,7 @@ public: void FillInfo(LogPersistQueryResult *response); int MkDirPath(const char *p_cMkdir); bool writeUnCompressedBuffer(HilogData *data); - uint8_t getType() const; + uint8_t GetType() const; std::string getPath(); LogPersisterBuffer *buffer; diff --git a/services/hilogd/include/log_querier.h b/services/hilogd/include/log_querier.h index e5f804e..5f89d8f 100644 --- a/services/hilogd/include/log_querier.h +++ b/services/hilogd/include/log_querier.h @@ -27,7 +27,7 @@ public: int WriteData(LogQueryResponse& rsp, HilogData* data); int WriteData(HilogData* data); void NotifyForNewData(); - uint8_t getType() const; + uint8_t GetType() const; ~LogQuerier() = default; }; } // namespace HiviewDFX diff --git a/services/hilogd/include/log_reader.h b/services/hilogd/include/log_reader.h index acdfe5f..24290fa 100644 --- a/services/hilogd/include/log_reader.h +++ b/services/hilogd/include/log_reader.h @@ -31,6 +31,7 @@ class HilogBuffer; #define TYPE_QUERIER 1 #define TYPE_PERSISTER 2 +#define TYPE_CONTROL 3 using QueryCondition = struct QueryCondition { uint8_t nPid = 0; @@ -70,7 +71,7 @@ public: virtual int WriteData(HilogData* data) =0; void SetSendId(unsigned int value); void SetCmd(uint8_t value); - virtual uint8_t getType() const = 0; + virtual uint8_t GetType() const = 0; protected: unsigned int sendId = 1; uint8_t cmd = 0; diff --git a/services/hilogd/log_collector.cpp b/services/hilogd/log_collector.cpp index 2429001..1a6531f 100644 --- a/services/hilogd/log_collector.cpp +++ b/services/hilogd/log_collector.cpp @@ -87,7 +87,9 @@ size_t LogCollector::InsertLogToBuffer(const HilogMsg& msg) hilogBuffer->logReaderListMutex.lock_shared(); auto it = hilogBuffer->logReaderList.begin(); while (it != hilogBuffer->logReaderList.end()) { - (*it).lock()->NotifyForNewData(); + if ((*it).lock()->GetType() != TYPE_CONTROL) { + (*it).lock()->NotifyForNewData(); + } ++it; } hilogBuffer->logReaderListMutex.unlock_shared(); diff --git a/services/hilogd/log_persister.cpp b/services/hilogd/log_persister.cpp index be8a760..481294f 100644 --- a/services/hilogd/log_persister.cpp +++ b/services/hilogd/log_persister.cpp @@ -408,7 +408,7 @@ string LogPersister::getPath() return path; } -uint8_t LogPersister::getType() const +uint8_t LogPersister::GetType() const { return TYPE_PERSISTER; } diff --git a/services/hilogd/log_querier.cpp b/services/hilogd/log_querier.cpp index 70251ce..8058f26 100644 --- a/services/hilogd/log_querier.cpp +++ b/services/hilogd/log_querier.cpp @@ -552,9 +552,16 @@ void LogQuerier::NotifyForNewData() } } -uint8_t LogQuerier::getType() const +uint8_t LogQuerier::GetType() const { - return TYPE_QUERIER; + switch (cmd) { + case LOG_QUERY_RESPONSE: + return TYPE_QUERIER; + case NEXT_RESPONSE: + return TYPE_QUERIER; + default: + return TYPE_CONTROL; + } } } // namespace HiviewDFX } // namespace OHOS -- Gitee From 78525381fe613be8295912afe58c146f48ee667c Mon Sep 17 00:00:00 2001 From: Wu Shangwei <2826256824@qq.com> Date: Thu, 8 Jul 2021 18:50:08 +0800 Subject: [PATCH 2/3] Avoid illegal parameters in domain and pid Signed-off-by: Wu Shangwei <2826256824@qq.com> Change-Id: I4cf32263354b6ef8f5758140badb1fa9bb9e4a70 --- services/hilogtool/include/hilogtool.h | 4 ++-- services/hilogtool/log_controller.cpp | 6 +++--- services/hilogtool/main.cpp | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/services/hilogtool/include/hilogtool.h b/services/hilogtool/include/hilogtool.h index b9138c4..e2e8abf 100644 --- a/services/hilogtool/include/hilogtool.h +++ b/services/hilogtool/include/hilogtool.h @@ -31,12 +31,12 @@ typedef struct { uint16_t tailLines; std::string domain; // domain recv std::string tag; // tag recv - uint32_t pids[MAX_PIDS]; + std::string pids[MAX_PIDS]; std::string domains[MAX_DOMAINS]; // domains send std::string tags[MAX_TAGS]; // tags send uint16_t noTypes; uint16_t noLevels; - uint32_t noPids[MAX_PIDS]; + std::string noPids[MAX_PIDS]; std::string noDomains[MAX_DOMAINS]; std::string noTags[MAX_TAGS]; std::string regexArgs; diff --git a/services/hilogtool/log_controller.cpp b/services/hilogtool/log_controller.cpp index 0f5415e..eb585a9 100644 --- a/services/hilogtool/log_controller.cpp +++ b/services/hilogtool/log_controller.cpp @@ -160,10 +160,10 @@ void LogQueryRequestOp(SeqPacketSocketClient& controller, const HilogArgs* conte logQueryRequest.nDomain = context->nDomain; logQueryRequest.nTag = context->nTag; for (int i = 0; i < context->nPid; i++) { - logQueryRequest.pids[i] = context->pids[i]; + std::istringstream(context->pids[i]) >> std::dec >> logQueryRequest.pids[i]; } for (int i = 0; i < context->nDomain; i++) { - logQueryRequest.domains[i] = stoul(context->domains[i]); + std::istringstream(context->domains[i]) >> std::hex >> logQueryRequest.domains[i]; } for (int i = 0; i < context->nTag; i++) { if (context->tags[i].length() >= MAX_TAG_LEN) { @@ -180,7 +180,7 @@ void LogQueryRequestOp(SeqPacketSocketClient& controller, const HilogArgs* conte logQueryRequest.nNoDomain = context->nNoDomain; logQueryRequest.nNoTag = context->nNoTag; for (int i = 0; i < context->nNoPid; i++) { - logQueryRequest.noPids[i] = context->noPids[i]; + std::istringstream(context->noPids[i]) >> std::dec >> logQueryRequest.noPids[i]; } for (int i = 0; i < context->nNoDomain; i++) { std::istringstream(context->noDomains[i]) >> std::hex >> logQueryRequest.noDomains[i]; diff --git a/services/hilogtool/main.cpp b/services/hilogtool/main.cpp index b2095c6..39de63e 100644 --- a/services/hilogtool/main.cpp +++ b/services/hilogtool/main.cpp @@ -423,13 +423,13 @@ int HilogEntry(int argc, char* argv[]) vector v(sregex_token_iterator(pids.begin() + 1, pids.end(), delimiter, -1), sregex_token_iterator()); for (auto s: v) { - context.noPids[context.nNoPid++] = stoul(s); + context.noPids[context.nNoPid++] = s; } } else { vector v(sregex_token_iterator(pids.begin(), pids.end(), delimiter, -1), sregex_token_iterator()); for (auto s: v) { - context.pids[context.nPid++] = stoul(s); + context.pids[context.nPid++] = s; context.pidArgs += s + " "; } } -- Gitee From 0fdd8548d0d455107d2cc6b0559e97d7943fb4a2 Mon Sep 17 00:00:00 2001 From: Wu Shangwei <2826256824@qq.com> Date: Thu, 8 Jul 2021 19:42:41 +0800 Subject: [PATCH 3/3] Fix deadlock bug in LogBuffer Signed-off-by: Wu Shangwei <2826256824@qq.com> Change-Id: Ibc9cb599d72f2c5e2efa3e04964064f07054d7ab --- services/hilogd/include/log_buffer.h | 1 + services/hilogd/include/log_reader.h | 2 +- services/hilogd/log_buffer.cpp | 13 +++++++++++++ services/hilogd/log_persister.cpp | 1 + services/hilogd/log_querier.cpp | 1 + services/hilogd/log_reader.cpp | 13 ------------- 6 files changed, 17 insertions(+), 14 deletions(-) diff --git a/services/hilogd/include/log_buffer.h b/services/hilogd/include/log_buffer.h index 7255074..6fd71d9 100644 --- a/services/hilogd/include/log_buffer.h +++ b/services/hilogd/include/log_buffer.h @@ -48,6 +48,7 @@ public: int32_t ClearStatisticInfoByDomain(uint32_t domain); void GetBufferLock(); void ReleaseBufferLock(); + void RemoveLogReader(std::shared_ptr reader); private: size_t size; size_t sizeByType[LOG_TYPE_MAX]; diff --git a/services/hilogd/include/log_reader.h b/services/hilogd/include/log_reader.h index 24290fa..0b767a5 100644 --- a/services/hilogd/include/log_reader.h +++ b/services/hilogd/include/log_reader.h @@ -62,7 +62,7 @@ public: bool isNotified; LogReader(); - virtual ~LogReader(); + virtual ~LogReader() = default; bool GetReload() const; void SetReload(bool); virtual void NotifyForNewData() = 0; diff --git a/services/hilogd/log_buffer.cpp b/services/hilogd/log_buffer.cpp index 7b63f57..a3a12fd 100644 --- a/services/hilogd/log_buffer.cpp +++ b/services/hilogd/log_buffer.cpp @@ -415,5 +415,18 @@ void HilogBuffer::ReleaseBufferLock() { hilogBufferMutex.unlock(); } + +void HilogBuffer::RemoveLogReader(std::shared_ptr reader) +{ + logReaderListMutex.lock(); + const auto findIter = std::find_if(logReaderList.begin(), logReaderList.end(), + [reader](const std::weak_ptr& ptr0) { + return ptr0.lock() == reader; + }); + if (findIter != logReaderList.end()) { + logReaderList.erase(findIter); + } + logReaderListMutex.unlock(); +} } // namespace HiviewDFX } // namespace OHOS diff --git a/services/hilogd/log_persister.cpp b/services/hilogd/log_persister.cpp index 481294f..441fc3c 100644 --- a/services/hilogd/log_persister.cpp +++ b/services/hilogd/log_persister.cpp @@ -327,6 +327,7 @@ int LogPersister::ThreadFunc() hasExited = true; } cvhasExited.notify_all(); + hilogBuffer->RemoveLogReader(shared_from_this()); return 0; } diff --git a/services/hilogd/log_querier.cpp b/services/hilogd/log_querier.cpp index 8058f26..ea6d7ee 100644 --- a/services/hilogd/log_querier.cpp +++ b/services/hilogd/log_querier.cpp @@ -488,6 +488,7 @@ void LogQuerier::LogQuerierThreadFunc(std::shared_ptr logReader) break; } } + hilogBuffer->RemoveLogReader(logReader); } LogQuerier::LogQuerier(std::unique_ptr handler, HilogBuffer* buffer) diff --git a/services/hilogd/log_reader.cpp b/services/hilogd/log_reader.cpp index cad3aa6..aa801c6 100644 --- a/services/hilogd/log_reader.cpp +++ b/services/hilogd/log_reader.cpp @@ -33,19 +33,6 @@ LogReader::LogReader() isNotified = false; } -LogReader::~LogReader() -{ - hilogBuffer->logReaderListMutex.lock(); - const auto findIter = std::find_if(hilogBuffer->logReaderList.begin(), hilogBuffer->logReaderList.end(), - [this](const std::weak_ptr& ptr0) { - return ptr0.lock() == weak_from_this().lock(); - }); - if (findIter != hilogBuffer->logReaderList.end()) { - hilogBuffer->logReaderList.erase(findIter); - } - hilogBuffer->logReaderListMutex.unlock(); -} - void LogReader::NotifyReload() { isReload = true; -- Gitee