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
diff --git a/logd/SimpleLogBuffer.cpp b/logd/SimpleLogBuffer.cpp
index 79ce069..5ab8e09 100644
--- a/logd/SimpleLogBuffer.cpp
+++ b/logd/SimpleLogBuffer.cpp
@@ -119,8 +119,12 @@
pid_t* last_tid() { return last_tid_; }
+ bool drop_chatty_messages() const { return drop_chatty_messages_; }
+ void set_drop_chatty_messages(bool value) { drop_chatty_messages_ = value; }
+
private:
pid_t last_tid_[LOG_ID_MAX] = {};
+ bool drop_chatty_messages_ = true;
};
std::unique_ptr<FlushToState> SimpleLogBuffer::CreateFlushToState(uint64_t start,
@@ -131,7 +135,7 @@
bool SimpleLogBuffer::FlushTo(
LogWriter* writer, FlushToState& abstract_state,
const std::function<FilterResult(log_id_t log_id, pid_t pid, uint64_t sequence,
- log_time realtime, uint16_t dropped_count)>& filter) {
+ log_time realtime)>& filter) {
auto shared_lock = SharedLock{lock_};
auto& state = reinterpret_cast<ChattyFlushToState&>(abstract_state);
@@ -169,8 +173,8 @@
}
if (filter) {
- FilterResult ret = filter(element.log_id(), element.pid(), element.sequence(),
- element.realtime(), element.dropped_count());
+ FilterResult ret =
+ filter(element.log_id(), element.pid(), element.sequence(), element.realtime());
if (ret == FilterResult::kSkip) {
continue;
}
@@ -179,6 +183,16 @@
}
}
+ // drop_chatty_messages is initialized to true, so if the first message that we attempt to
+ // flush is a chatty message, we drop it. Once we see a non-chatty message it gets set to
+ // false to let further chatty messages be printed.
+ if (state.drop_chatty_messages()) {
+ if (element.dropped_count() != 0) {
+ continue;
+ }
+ state.set_drop_chatty_messages(false);
+ }
+
bool same_tid = state.last_tid()[element.log_id()] == element.tid();
// Dropped (chatty) immediately following a valid log from the same source in the same log
// buffer indicates we have a multiple identical squash. chatty that differs source is due