summaryrefslogtreecommitdiff
path: root/logd/LogReaderThread.cpp
diff options
context:
space:
mode:
authorTom Cherry <tomcherry@google.com>2020-05-28 20:02:42 -0700
committerTom Cherry <tomcherry@google.com>2020-06-02 13:26:48 -0700
commitb3e163399a9038175d46c9dbd9262d009e87ceb6 (patch)
treecb63225fcd72980db33138e17d98e6f682a25e45 /logd/LogReaderThread.cpp
parent4ab0bc414d8a01df2013ea70f8e10f5daa12bec4 (diff)
logd: move leading_dropped logic into FlushTo()
This logic isn't generic, so it should not be in the generic LogReaderThread. Moreover, it's currently broken in essentially every case except when filtering by UID, because it runs as in the filter functions before the actual filtering by pid/etc takes place. For example, when filtering by pid, it's possible to get leading chatty messages. The newly added test was failing previously but is fixed by this change. It's fundamentally broken in the tail case. Take this example: 1: Normal message 2: Chatty message 3: Normal message 4: Normal message If you read that log buffer with a tail value of 3, there are three possible outcomes: 1) Messages #2-4, however this would include a leading chatty message, which is not allowed. 2) Messages #3-4, however this is only 2, not 3 messages. 3) Messages #1-4, however this is 4, more than the 3 requested messages. This code chooses 2) as the correct solution, in this case, we don't need to account for leading chatty messages when counting the total logs in the buffer. A test is added for this case as well. Test: new unit test Change-Id: Id02eb81a8e77390aba4f85aac659c6cab498dbcd
Diffstat (limited to 'logd/LogReaderThread.cpp')
-rw-r--r--logd/LogReaderThread.cpp41
1 files changed, 9 insertions, 32 deletions
diff --git a/logd/LogReaderThread.cpp b/logd/LogReaderThread.cpp
index c6e60feff6..dc8582d1a4 100644
--- a/logd/LogReaderThread.cpp
+++ b/logd/LogReaderThread.cpp
@@ -35,7 +35,6 @@ LogReaderThread::LogReaderThread(LogBuffer* log_buffer, LogReaderList* reader_li
: log_buffer_(log_buffer),
reader_list_(reader_list),
writer_(std::move(writer)),
- leading_dropped_(false),
pid_(pid),
tail_(tail),
count_(0),
@@ -52,8 +51,6 @@ LogReaderThread::LogReaderThread(LogBuffer* log_buffer, LogReaderList* reader_li
void LogReaderThread::ThreadFunction() {
prctl(PR_SET_NAME, "logd.reader.per");
- leading_dropped_ = true;
-
auto lock = std::unique_lock{reader_list_->reader_threads_lock()};
while (!release_) {
@@ -72,21 +69,16 @@ void LogReaderThread::ThreadFunction() {
if (tail_) {
auto first_pass_state = log_buffer_->CreateFlushToState(flush_to_state_->start(),
flush_to_state_->log_mask());
- log_buffer_->FlushTo(writer_.get(), *first_pass_state,
- [this](log_id_t log_id, pid_t pid, uint64_t sequence,
- log_time realtime, uint16_t dropped_count) {
- return FilterFirstPass(log_id, pid, sequence, realtime,
- dropped_count);
- });
- leading_dropped_ =
- true; // TODO: Likely a bug, if leading_dropped_ was not true before calling
- // flushTo(), then it should not be reset to true after.
+ log_buffer_->FlushTo(
+ writer_.get(), *first_pass_state,
+ [this](log_id_t log_id, pid_t pid, uint64_t sequence, log_time realtime) {
+ return FilterFirstPass(log_id, pid, sequence, realtime);
+ });
}
bool flush_success = log_buffer_->FlushTo(
writer_.get(), *flush_to_state_,
- [this](log_id_t log_id, pid_t pid, uint64_t sequence, log_time realtime,
- uint16_t dropped_count) {
- return FilterSecondPass(log_id, pid, sequence, realtime, dropped_count);
+ [this](log_id_t log_id, pid_t pid, uint64_t sequence, log_time realtime) {
+ return FilterSecondPass(log_id, pid, sequence, realtime);
});
// We only ignore entries before the original start time for the first flushTo(), if we
@@ -127,17 +119,9 @@ void LogReaderThread::ThreadFunction() {
}
// A first pass to count the number of elements
-FilterResult LogReaderThread::FilterFirstPass(log_id_t, pid_t pid, uint64_t, log_time realtime,
- uint16_t dropped_count) {
+FilterResult LogReaderThread::FilterFirstPass(log_id_t, pid_t pid, uint64_t, log_time realtime) {
auto lock = std::lock_guard{reader_list_->reader_threads_lock()};
- if (leading_dropped_) {
- if (dropped_count) {
- return FilterResult::kSkip;
- }
- leading_dropped_ = false;
- }
-
if ((!pid_ || pid_ == pid) && (start_time_ == log_time::EPOCH || start_time_ <= realtime)) {
++count_;
}
@@ -147,7 +131,7 @@ FilterResult LogReaderThread::FilterFirstPass(log_id_t, pid_t pid, uint64_t, log
// A second pass to send the selected elements
FilterResult LogReaderThread::FilterSecondPass(log_id_t log_id, pid_t pid, uint64_t,
- log_time realtime, uint16_t dropped_count) {
+ log_time realtime) {
auto lock = std::lock_guard{reader_list_->reader_threads_lock()};
if (skip_ahead_[log_id]) {
@@ -155,13 +139,6 @@ FilterResult LogReaderThread::FilterSecondPass(log_id_t log_id, pid_t pid, uint6
return FilterResult::kSkip;
}
- if (leading_dropped_) {
- if (dropped_count) {
- return FilterResult::kSkip;
- }
- leading_dropped_ = false;
- }
-
// Truncate to close race between first and second pass
if (non_block_ && tail_ && index_ >= count_) {
return FilterResult::kStop;