Revert "logd: drop mSequence from LogBufferElement"
This reverts commit 5a34d6ea43d28f3b5d27bf6dd5b9fa31ec033531.
There is a long standing bug that logd will leak memory during its
prune process if the time on the device changes significantly forwards
then backwards. This is due to using the timestamp of each log
message to determine what log messages are yet to be processed by a
reader thread.
Various attempts have been made to rectify this, but the only solution
that safely fixes this issue is to go back to using sequence numbers
on the log messages.
Bug: 64675203
Bug: 77971811
Bug: 149340579
Bug: 150923384
Test: logcat output looks sane
Change-Id: Ibce79cf184eb29a4914f3e42a8cb2868d04dc165
diff --git a/logd/LogReader.cpp b/logd/LogReader.cpp
index d52d459..22178f1 100644
--- a/logd/LogReader.cpp
+++ b/logd/LogReader.cpp
@@ -129,70 +129,53 @@
nonBlock = true;
}
- log_time sequence = start;
- //
- // This somewhat expensive data validation operation is required
- // for non-blocking, with timeout. The incoming timestamp must be
- // in range of the list, if not, return immediately. This is
- // used to prevent us from from getting stuck in timeout processing
- // with an invalid time.
- //
- // Find if time is really present in the logs, monotonic or real, implicit
- // conversion from monotonic or real as necessary to perform the check.
- // Exit in the check loop ASAP as you find a transition from older to
- // newer, but use the last entry found to ensure overlap.
- //
- if (nonBlock && (sequence != log_time::EPOCH) && timeout) {
- class LogFindStart { // A lambda by another name
- private:
+ uint64_t sequence = 1;
+ // Convert realtime to sequence number
+ if (start != log_time::EPOCH) {
+ class LogFindStart {
const pid_t mPid;
const unsigned mLogMask;
- bool mStartTimeSet;
- log_time mStart;
- log_time& mSequence;
- log_time mLast;
- bool mIsMonotonic;
+ bool startTimeSet;
+ log_time& start;
+ uint64_t& sequence;
+ uint64_t last;
+ bool isMonotonic;
- public:
- LogFindStart(pid_t pid, unsigned logMask, log_time& sequence,
+ public:
+ LogFindStart(unsigned logMask, pid_t pid, log_time& start, uint64_t& sequence,
bool isMonotonic)
: mPid(pid),
mLogMask(logMask),
- mStartTimeSet(false),
- mStart(sequence),
- mSequence(sequence),
- mLast(sequence),
- mIsMonotonic(isMonotonic) {
- }
+ startTimeSet(false),
+ start(start),
+ sequence(sequence),
+ last(sequence),
+ isMonotonic(isMonotonic) {}
static int callback(const LogBufferElement* element, void* obj) {
LogFindStart* me = reinterpret_cast<LogFindStart*>(obj);
if ((!me->mPid || (me->mPid == element->getPid())) &&
(me->mLogMask & (1 << element->getLogId()))) {
- log_time real = element->getRealTime();
- if (me->mStart == real) {
- me->mSequence = real;
- me->mStartTimeSet = true;
+ if (me->start == element->getRealTime()) {
+ me->sequence = element->getSequence();
+ me->startTimeSet = true;
return -1;
- } else if (!me->mIsMonotonic || android::isMonotonic(real)) {
- if (me->mStart < real) {
- me->mSequence = me->mLast;
- me->mStartTimeSet = true;
+ } else if (!me->isMonotonic || android::isMonotonic(element->getRealTime())) {
+ if (me->start < element->getRealTime()) {
+ me->sequence = me->last;
+ me->startTimeSet = true;
return -1;
}
- me->mLast = real;
+ me->last = element->getSequence();
} else {
- me->mLast = real;
+ me->last = element->getSequence();
}
}
return false;
}
- bool found() {
- return mStartTimeSet;
- }
-
- } logFindStart(pid, logMask, sequence,
+ bool found() { return startTimeSet; }
+ } logFindStart(logMask, pid, start, sequence,
logbuf().isMonotonic() && android::isMonotonic(start));
logbuf().flushTo(cli, sequence, nullptr, FlushCommand::hasReadLogs(cli),
@@ -200,18 +183,21 @@
logFindStart.callback, &logFindStart);
if (!logFindStart.found()) {
- doSocketDelete(cli);
- return false;
+ if (nonBlock) {
+ doSocketDelete(cli);
+ return false;
+ }
+ sequence = LogBufferElement::getCurrentSequence();
}
}
android::prdebug(
- "logdr: UID=%d GID=%d PID=%d %c tail=%lu logMask=%x pid=%d "
- "start=%" PRIu64 "ns timeout=%" PRIu64 "ns\n",
- cli->getUid(), cli->getGid(), cli->getPid(), nonBlock ? 'n' : 'b', tail,
- logMask, (int)pid, sequence.nsec(), timeout);
+ "logdr: UID=%d GID=%d PID=%d %c tail=%lu logMask=%x pid=%d "
+ "start=%" PRIu64 "ns timeout=%" PRIu64 "ns\n",
+ cli->getUid(), cli->getGid(), cli->getPid(), nonBlock ? 'n' : 'b', tail, logMask,
+ (int)pid, start.nsec(), timeout);
- if (sequence == log_time::EPOCH) {
+ if (start == log_time::EPOCH) {
timeout = 0;
}