From 6ec71e925345855a5d94902fe6490eba80799910 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Mon, 4 May 2020 12:53:36 -0700 Subject: [PATCH] logd: rename LogTimes -> LogReaderThread LogTimes has evolved from being simply a store of the last timestamp that each reader has read to being a class representing an individual reader thread, including the thread function, so name it appropriately. Test: logging unit tests Change-Id: I6914824376a6ff1f7509e657fa4dc044ead62954 --- logd/Android.bp | 2 +- logd/LogBuffer.cpp | 24 ++++----- logd/LogBuffer.h | 4 +- logd/LogReader.cpp | 31 ++++++------ logd/LogReader.h | 7 +-- logd/{LogTimes.cpp => LogReaderThread.cpp} | 57 +++++++++++----------- logd/{LogTimes.h => LogReaderThread.h} | 47 ++++++------------ logd/fuzz/log_buffer_log_fuzzer.cpp | 2 +- 8 files changed, 77 insertions(+), 97 deletions(-) rename logd/{LogTimes.cpp => LogReaderThread.cpp} (78%) rename logd/{LogTimes.h => LogReaderThread.h} (64%) diff --git a/logd/Android.bp b/logd/Android.bp index 26632713d..b6d30cde8 100644 --- a/logd/Android.bp +++ b/logd/Android.bp @@ -36,9 +36,9 @@ cc_library_static { "CommandListener.cpp", "LogListener.cpp", "LogReader.cpp", + "LogReaderThread.cpp", "LogBuffer.cpp", "LogBufferElement.cpp", - "LogTimes.cpp", "LogStatistics.cpp", "LogWhiteBlackList.cpp", "libaudit.c", diff --git a/logd/LogBuffer.cpp b/logd/LogBuffer.cpp index a3e4e094e..f2d247e4b 100644 --- a/logd/LogBuffer.cpp +++ b/logd/LogBuffer.cpp @@ -93,16 +93,16 @@ void LogBuffer::init() { } // Release any sleeping reader threads to dump their current content. - LogTimeEntry::wrlock(); + LogReaderThread::wrlock(); LastLogTimes::iterator times = mTimes.begin(); while (times != mTimes.end()) { - LogTimeEntry* entry = times->get(); + LogReaderThread* entry = times->get(); entry->triggerReader_Locked(); times++; } - LogTimeEntry::unlock(); + LogReaderThread::unlock(); } LogBuffer::LogBuffer(LastLogTimes* times, LogTags* tags, PruneList* prune) @@ -579,7 +579,7 @@ class LogBufferElementLast { // If the selected reader is blocking our pruning progress, decide on // what kind of mitigation is necessary to unblock the situation. -void LogBuffer::kickMe(LogTimeEntry* me, log_id_t id, unsigned long pruneRows) { +void LogBuffer::kickMe(LogReaderThread* me, log_id_t id, unsigned long pruneRows) { if (stats.sizes(id) > (2 * log_buffer_size(id))) { // +100% // A misbehaving or slow reader has its connection // dropped if we hit too much memory pressure. @@ -647,16 +647,16 @@ void LogBuffer::kickMe(LogTimeEntry* me, log_id_t id, unsigned long pruneRows) { // LogBuffer::wrlock() must be held when this function is called. // bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) { - LogTimeEntry* oldest = nullptr; + LogReaderThread* oldest = nullptr; bool busy = false; bool clearAll = pruneRows == ULONG_MAX; - LogTimeEntry::rdlock(); + LogReaderThread::rdlock(); // Region locked? LastLogTimes::iterator times = mTimes.begin(); while (times != mTimes.end()) { - LogTimeEntry* entry = times->get(); + LogReaderThread* entry = times->get(); if (entry->isWatching(id) && (!oldest || (oldest->mStart > entry->mStart) || ((oldest->mStart == entry->mStart) && @@ -692,7 +692,7 @@ bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) { break; } } - LogTimeEntry::unlock(); + LogReaderThread::unlock(); return busy; } @@ -953,7 +953,7 @@ bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) { } } - LogTimeEntry::unlock(); + LogReaderThread::unlock(); return (pruneRows > 0) && busy; } @@ -976,10 +976,10 @@ bool LogBuffer::clear(log_id_t id, uid_t uid) { // readers and let the clear run (below) deal with determining // if we are still blocked and return an error code to caller. if (busy) { - LogTimeEntry::wrlock(); + LogReaderThread::wrlock(); LastLogTimes::iterator times = mTimes.begin(); while (times != mTimes.end()) { - LogTimeEntry* entry = times->get(); + LogReaderThread* entry = times->get(); // Killer punch if (entry->isWatching(id)) { android::prdebug( @@ -989,7 +989,7 @@ bool LogBuffer::clear(log_id_t id, uid_t uid) { } times++; } - LogTimeEntry::unlock(); + LogReaderThread::unlock(); } } wrlock(); diff --git a/logd/LogBuffer.h b/logd/LogBuffer.h index 9a367127a..09efc5193 100644 --- a/logd/LogBuffer.h +++ b/logd/LogBuffer.h @@ -27,9 +27,9 @@ #include #include "LogBufferElement.h" +#include "LogReaderThread.h" #include "LogStatistics.h" #include "LogTags.h" -#include "LogTimes.h" #include "LogWhiteBlackList.h" // @@ -152,7 +152,7 @@ class LogBuffer { static constexpr size_t maxPrune = 256; void maybePrune(log_id_t id); - void kickMe(LogTimeEntry* me, log_id_t id, unsigned long pruneRows); + void kickMe(LogReaderThread* me, log_id_t id, unsigned long pruneRows); bool prune(log_id_t id, unsigned long pruneRows, uid_t uid = AID_ROOT); LogBufferElementCollection::iterator erase( diff --git a/logd/LogReader.cpp b/logd/LogReader.cpp index c6dea69f8..441f85b80 100644 --- a/logd/LogReader.cpp +++ b/logd/LogReader.cpp @@ -42,7 +42,7 @@ LogReader::LogReader(LogBuffer* logbuf) void LogReader::notifyNewLog(log_mask_t log_mask) { LastLogTimes& times = mLogbuf.mTimes; - LogTimeEntry::wrlock(); + LogReaderThread::wrlock(); for (const auto& entry : times) { if (!entry->isWatchingMultiple(log_mask)) { continue; @@ -52,7 +52,7 @@ void LogReader::notifyNewLog(log_mask_t log_mask) { } entry->triggerReader_Locked(); } - LogTimeEntry::unlock(); + LogReaderThread::unlock(); } // Note returning false will release the SocketClient instance. @@ -74,15 +74,15 @@ bool LogReader::onDataAvailable(SocketClient* cli) { // Clients are only allowed to send one command, disconnect them if they // send another. - LogTimeEntry::wrlock(); + LogReaderThread::wrlock(); for (const auto& entry : mLogbuf.mTimes) { if (entry->mClient == cli) { entry->release_Locked(); - LogTimeEntry::unlock(); + LogReaderThread::unlock(); return false; } } - LogTimeEntry::unlock(); + LogReaderThread::unlock(); unsigned long tail = 0; static const char _tail[] = " tail="; @@ -137,8 +137,8 @@ bool LogReader::onDataAvailable(SocketClient* cli) { if (!fastcmp(buffer, "dumpAndClose", 12)) { // Allow writer to get some cycles, and wait for pending notifications sched_yield(); - LogTimeEntry::wrlock(); - LogTimeEntry::unlock(); + LogReaderThread::wrlock(); + LogReaderThread::unlock(); sched_yield(); nonBlock = true; } @@ -217,11 +217,12 @@ bool LogReader::onDataAvailable(SocketClient* cli) { timeout = 0; } - LogTimeEntry::wrlock(); - auto entry = std::make_unique(*this, cli, nonBlock, tail, logMask, pid, start, - sequence, timeout, privileged, can_read_security); + LogReaderThread::wrlock(); + auto entry = + std::make_unique(*this, cli, nonBlock, tail, logMask, pid, start, + sequence, timeout, privileged, can_read_security); if (!entry->startReader_Locked()) { - LogTimeEntry::unlock(); + LogReaderThread::unlock(); return false; } @@ -234,24 +235,24 @@ bool LogReader::onDataAvailable(SocketClient* cli) { setsockopt(cli->getSocket(), SOL_SOCKET, SO_SNDTIMEO, (const char*)&t, sizeof(t)); - LogTimeEntry::unlock(); + LogReaderThread::unlock(); return true; } void LogReader::doSocketDelete(SocketClient* cli) { LastLogTimes& times = mLogbuf.mTimes; - LogTimeEntry::wrlock(); + LogReaderThread::wrlock(); LastLogTimes::iterator it = times.begin(); while (it != times.end()) { - LogTimeEntry* entry = it->get(); + LogReaderThread* entry = it->get(); if (entry->mClient == cli) { entry->release_Locked(); break; } it++; } - LogTimeEntry::unlock(); + LogReaderThread::unlock(); } int LogReader::getLogSocket() { diff --git a/logd/LogReader.h b/logd/LogReader.h index b5312b60d..e50ca8e82 100644 --- a/logd/LogReader.h +++ b/logd/LogReader.h @@ -14,12 +14,11 @@ * limitations under the License. */ -#ifndef _LOGD_LOG_WRITER_H__ -#define _LOGD_LOG_WRITER_H__ +#pragma once #include -#include "LogTimes.h" +#include "LogReaderThread.h" #define LOGD_SNDTIMEO 32 @@ -44,5 +43,3 @@ class LogReader : public SocketListener { void doSocketDelete(SocketClient* cli); }; - -#endif diff --git a/logd/LogTimes.cpp b/logd/LogReaderThread.cpp similarity index 78% rename from logd/LogTimes.cpp rename to logd/LogReaderThread.cpp index ad150bdb6..f1452a663 100644 --- a/logd/LogTimes.cpp +++ b/logd/LogReaderThread.cpp @@ -14,23 +14,24 @@ * limitations under the License. */ +#include "LogReaderThread.h" + #include #include #include #include "LogBuffer.h" #include "LogReader.h" -#include "LogTimes.h" -pthread_mutex_t LogTimeEntry::timesLock = PTHREAD_MUTEX_INITIALIZER; +pthread_mutex_t LogReaderThread::timesLock = PTHREAD_MUTEX_INITIALIZER; -LogTimeEntry::LogTimeEntry(LogReader& reader, SocketClient* client, bool nonBlock, - unsigned long tail, log_mask_t logMask, pid_t pid, log_time start_time, - uint64_t start, uint64_t timeout, bool privileged, - bool can_read_security_logs) +LogReaderThread::LogReaderThread(LogReader& reader, SocketClient* client, bool non_block, + unsigned long tail, log_mask_t log_mask, pid_t pid, + log_time start_time, uint64_t start, uint64_t timeout, + bool privileged, bool can_read_security_logs) : leadingDropped(false), mReader(reader), - mLogMask(logMask), + mLogMask(log_mask), mPid(pid), mCount(0), mTail(tail), @@ -38,7 +39,7 @@ LogTimeEntry::LogTimeEntry(LogReader& reader, SocketClient* client, bool nonBloc mClient(client), mStartTime(start_time), mStart(start), - mNonBlock(nonBlock), + mNonBlock(non_block), privileged_(privileged), can_read_security_logs_(can_read_security_logs) { mTimeout.tv_sec = timeout / NS_PER_SEC; @@ -48,13 +49,12 @@ LogTimeEntry::LogTimeEntry(LogReader& reader, SocketClient* client, bool nonBloc cleanSkip_Locked(); } -bool LogTimeEntry::startReader_Locked() { +bool LogReaderThread::startReader_Locked() { pthread_attr_t attr; if (!pthread_attr_init(&attr)) { if (!pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED)) { - if (!pthread_create(&mThread, &attr, LogTimeEntry::threadStart, - this)) { + if (!pthread_create(&mThread, &attr, LogReaderThread::threadStart, this)) { pthread_attr_destroy(&attr); return true; } @@ -65,10 +65,10 @@ bool LogTimeEntry::startReader_Locked() { return false; } -void* LogTimeEntry::threadStart(void* obj) { +void* LogReaderThread::threadStart(void* obj) { prctl(PR_SET_NAME, "logd.reader.per"); - LogTimeEntry* me = reinterpret_cast(obj); + LogReaderThread* me = reinterpret_cast(obj); SocketClient* client = me->mClient; @@ -136,9 +136,8 @@ void* LogTimeEntry::threadStart(void* obj) { client->decRef(); LastLogTimes& times = reader.logbuf().mTimes; - auto it = - std::find_if(times.begin(), times.end(), - [&me](const auto& other) { return other.get() == me; }); + auto it = std::find_if(times.begin(), times.end(), + [&me](const auto& other) { return other.get() == me; }); if (it != times.end()) { times.erase(it); @@ -150,14 +149,14 @@ void* LogTimeEntry::threadStart(void* obj) { } // A first pass to count the number of elements -int LogTimeEntry::FilterFirstPass(const LogBufferElement* element, void* obj) { - LogTimeEntry* me = reinterpret_cast(obj); +int LogReaderThread::FilterFirstPass(const LogBufferElement* element, void* obj) { + LogReaderThread* me = reinterpret_cast(obj); - LogTimeEntry::wrlock(); + LogReaderThread::wrlock(); if (me->leadingDropped) { if (element->getDropped()) { - LogTimeEntry::unlock(); + LogReaderThread::unlock(); return false; } me->leadingDropped = false; @@ -172,16 +171,16 @@ int LogTimeEntry::FilterFirstPass(const LogBufferElement* element, void* obj) { ++me->mCount; } - LogTimeEntry::unlock(); + LogReaderThread::unlock(); return false; } // A second pass to send the selected elements -int LogTimeEntry::FilterSecondPass(const LogBufferElement* element, void* obj) { - LogTimeEntry* me = reinterpret_cast(obj); +int LogReaderThread::FilterSecondPass(const LogBufferElement* element, void* obj) { + LogReaderThread* me = reinterpret_cast(obj); - LogTimeEntry::wrlock(); + LogReaderThread::wrlock(); me->mStart = element->getSequence(); @@ -234,20 +233,20 @@ int LogTimeEntry::FilterSecondPass(const LogBufferElement* element, void* obj) { ok: if (!me->skipAhead[element->getLogId()]) { - LogTimeEntry::unlock(); + LogReaderThread::unlock(); return true; } -// FALLTHRU + // FALLTHRU skip: - LogTimeEntry::unlock(); + LogReaderThread::unlock(); return false; stop: - LogTimeEntry::unlock(); + LogReaderThread::unlock(); return -1; } -void LogTimeEntry::cleanSkip_Locked(void) { +void LogReaderThread::cleanSkip_Locked(void) { memset(skipAhead, 0, sizeof(skipAhead)); } diff --git a/logd/LogTimes.h b/logd/LogReaderThread.h similarity index 64% rename from logd/LogTimes.h rename to logd/LogReaderThread.h index 56c930a15..b6a489d84 100644 --- a/logd/LogTimes.h +++ b/logd/LogReaderThread.h @@ -14,8 +14,7 @@ * limitations under the License. */ -#ifndef _LOGD_LOG_TIMES_H__ -#define _LOGD_LOG_TIMES_H__ +#pragma once #include #include @@ -33,7 +32,7 @@ typedef unsigned int log_mask_t; class LogReader; class LogBufferElement; -class LogTimeEntry { +class LogReaderThread { static pthread_mutex_t timesLock; bool mRelease = false; bool leadingDropped; @@ -50,9 +49,9 @@ class LogTimeEntry { unsigned long mIndex; public: - LogTimeEntry(LogReader& reader, SocketClient* client, bool nonBlock, unsigned long tail, - log_mask_t logMask, pid_t pid, log_time start_time, uint64_t sequence, - uint64_t timeout, bool privileged, bool can_read_security_logs); + LogReaderThread(LogReader& reader, SocketClient* client, bool non_block, unsigned long tail, + log_mask_t log_mask, pid_t pid, log_time start_time, uint64_t sequence, + uint64_t timeout, bool privileged, bool can_read_security_logs); SocketClient* mClient; log_time mStartTime; @@ -61,40 +60,26 @@ class LogTimeEntry { const bool mNonBlock; // Protect List manipulations - static void wrlock(void) { - pthread_mutex_lock(×Lock); - } - static void rdlock(void) { - pthread_mutex_lock(×Lock); - } - static void unlock(void) { - pthread_mutex_unlock(×Lock); - } + static void wrlock() { pthread_mutex_lock(×Lock); } + static void rdlock() { pthread_mutex_lock(×Lock); } + static void unlock() { pthread_mutex_unlock(×Lock); } bool startReader_Locked(); - void triggerReader_Locked(void) { - pthread_cond_signal(&threadTriggeredCondition); - } + void triggerReader_Locked() { pthread_cond_signal(&threadTriggeredCondition); } - void triggerSkip_Locked(log_id_t id, unsigned int skip) { - skipAhead[id] = skip; - } - void cleanSkip_Locked(void); + void triggerSkip_Locked(log_id_t id, unsigned int skip) { skipAhead[id] = skip; } + void cleanSkip_Locked(); - void release_Locked(void) { + void release_Locked() { // gracefully shut down the socket. shutdown(mClient->getSocket(), SHUT_RDWR); mRelease = true; pthread_cond_signal(&threadTriggeredCondition); } - bool isWatching(log_id_t id) const { - return mLogMask & (1 << id); - } - bool isWatchingMultiple(log_mask_t logMask) const { - return mLogMask & logMask; - } + bool isWatching(log_id_t id) const { return mLogMask & (1 << id); } + bool isWatchingMultiple(log_mask_t log_mask) const { return mLogMask & log_mask; } // flushTo filter callbacks static int FilterFirstPass(const LogBufferElement* element, void* me); static int FilterSecondPass(const LogBufferElement* element, void* me); @@ -104,6 +89,4 @@ class LogTimeEntry { bool can_read_security_logs_; }; -typedef std::list> LastLogTimes; - -#endif // _LOGD_LOG_TIMES_H__ +typedef std::list> LastLogTimes; diff --git a/logd/fuzz/log_buffer_log_fuzzer.cpp b/logd/fuzz/log_buffer_log_fuzzer.cpp index 14c5163e2..8156612da 100644 --- a/logd/fuzz/log_buffer_log_fuzzer.cpp +++ b/logd/fuzz/log_buffer_log_fuzzer.cpp @@ -16,7 +16,7 @@ #include #include "../LogBuffer.h" -#include "../LogTimes.h" +#include "../LogReaderThread.h" // We don't want to waste a lot of entropy on messages #define MAX_MSG_LENGTH 5