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/ChattyLogBufferTest.cpp b/logd/ChattyLogBufferTest.cpp
index 8754b88..3d9005a 100644
--- a/logd/ChattyLogBufferTest.cpp
+++ b/logd/ChattyLogBufferTest.cpp
@@ -202,4 +202,134 @@
CompareLogMessages(expected_log_messages, read_log_messages);
};
+TEST_P(ChattyLogBufferTest, no_leading_chatty_simple) {
+ auto make_message = [&](uint32_t sec, int32_t pid, uint32_t uid, uint32_t lid, const char* tag,
+ const char* msg, bool regex = false) -> LogMessage {
+ logger_entry entry = {.pid = pid, .tid = 1, .sec = sec, .nsec = 1, .lid = lid, .uid = uid};
+ std::string message;
+ message.push_back(ANDROID_LOG_INFO);
+ message.append(tag);
+ message.push_back('\0');
+ message.append(msg);
+ message.push_back('\0');
+ return {entry, message, regex};
+ };
+
+ // clang-format off
+ std::vector<LogMessage> log_messages = {
+ make_message(1, 1, 1, LOG_ID_MAIN, "test_tag", "duplicate1"),
+ make_message(2, 2, 2, LOG_ID_SYSTEM, "test_tag", "duplicate2"),
+ make_message(3, 2, 2, LOG_ID_SYSTEM, "test_tag", "duplicate2"),
+ make_message(4, 2, 2, LOG_ID_SYSTEM, "test_tag", "duplicate2"),
+ make_message(6, 2, 2, LOG_ID_SYSTEM, "test_tag", "not duplicate2"),
+ make_message(7, 1, 1, LOG_ID_MAIN, "test_tag", "duplicate1"),
+ make_message(8, 1, 1, LOG_ID_MAIN, "test_tag", "duplicate1"),
+ make_message(9, 1, 1, LOG_ID_MAIN, "test_tag", "duplicate1"),
+ make_message(10, 1, 1, LOG_ID_MAIN, "test_tag", "not duplicate1"),
+ };
+ // clang-format on
+ FixupMessages(&log_messages);
+ LogMessages(log_messages);
+
+ // After logging log_messages, the below is what should be in the buffer:
+ // PID=1, LOG_ID_MAIN duplicate1
+ // [1] PID=2, LOG_ID_SYSTEM duplicate2
+ // PID=2, LOG_ID_SYSTEM chatty drop
+ // PID=2, LOG_ID_SYSTEM duplicate2
+ // PID=2, LOG_ID_SYSTEM not duplicate2
+ // [2] PID=1, LOG_ID_MAIN chatty drop
+ // [3] PID=1, LOG_ID_MAIN duplicate1
+ // PID=1, LOG_ID_MAIN not duplicate1
+
+ // We then read from the 2nd sequence number, starting from log message [1], but filtering out
+ // everything but PID=1, which results in us starting with log message [2], which is a chatty
+ // drop. Code prior to this test case would erroneously print it. The intended behavior that
+ // this test checks prints logs starting from log message [3].
+
+ // clang-format off
+ std::vector<LogMessage> expected_log_messages = {
+ make_message(9, 1, 1, LOG_ID_MAIN, "test_tag", "duplicate1"),
+ make_message(10, 1, 1, LOG_ID_MAIN, "test_tag", "not duplicate1"),
+ };
+ FixupMessages(&expected_log_messages);
+ // clang-format on
+
+ std::vector<LogMessage> read_log_messages;
+ bool released = false;
+ {
+ auto lock = std::unique_lock{reader_list_.reader_threads_lock()};
+ std::unique_ptr<LogWriter> test_writer(new TestWriter(&read_log_messages, &released));
+ std::unique_ptr<LogReaderThread> log_reader(
+ new LogReaderThread(log_buffer_.get(), &reader_list_, std::move(test_writer), true,
+ 0, ~0, 1, {}, 2, {}));
+ reader_list_.reader_threads().emplace_back(std::move(log_reader));
+ }
+
+ while (!released) {
+ usleep(5000);
+ }
+
+ CompareLogMessages(expected_log_messages, read_log_messages);
+}
+
+TEST_P(ChattyLogBufferTest, no_leading_chatty_tail) {
+ auto make_message = [&](uint32_t sec, const char* tag, const char* msg,
+ bool regex = false) -> LogMessage {
+ logger_entry entry = {
+ .pid = 1, .tid = 1, .sec = sec, .nsec = 1, .lid = LOG_ID_MAIN, .uid = 0};
+ std::string message;
+ message.push_back(ANDROID_LOG_INFO);
+ message.append(tag);
+ message.push_back('\0');
+ message.append(msg);
+ message.push_back('\0');
+ return {entry, message, regex};
+ };
+
+ // clang-format off
+ std::vector<LogMessage> log_messages = {
+ make_message(1, "test_tag", "duplicate"),
+ make_message(2, "test_tag", "duplicate"),
+ make_message(3, "test_tag", "duplicate"),
+ make_message(4, "test_tag", "not_duplicate"),
+ };
+ // clang-format on
+ FixupMessages(&log_messages);
+ LogMessages(log_messages);
+
+ // After logging log_messages, the below is what should be in the buffer:
+ // "duplicate"
+ // chatty
+ // "duplicate"
+ // "not duplicate"
+
+ // We then read the tail 3 messages expecting there to not be a chatty message, meaning that we
+ // should only see the last two messages.
+
+ // clang-format off
+ std::vector<LogMessage> expected_log_messages = {
+ make_message(3, "test_tag", "duplicate"),
+ make_message(4, "test_tag", "not_duplicate"),
+ };
+ FixupMessages(&expected_log_messages);
+ // clang-format on
+
+ std::vector<LogMessage> read_log_messages;
+ bool released = false;
+ {
+ auto lock = std::unique_lock{reader_list_.reader_threads_lock()};
+ std::unique_ptr<LogWriter> test_writer(new TestWriter(&read_log_messages, &released));
+ std::unique_ptr<LogReaderThread> log_reader(
+ new LogReaderThread(log_buffer_.get(), &reader_list_, std::move(test_writer), true,
+ 3, ~0, 0, {}, 1, {}));
+ reader_list_.reader_threads().emplace_back(std::move(log_reader));
+ }
+
+ while (!released) {
+ usleep(5000);
+ }
+
+ CompareLogMessages(expected_log_messages, read_log_messages);
+}
+
INSTANTIATE_TEST_CASE_P(ChattyLogBufferTests, ChattyLogBufferTest, testing::Values("chatty"));