Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 10d086e2 authored by Tom Cherry's avatar Tom Cherry
Browse files

Revert "logd: drop mSequence from LogBufferElement" 

This reverts commit 5a34d6ea.

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
parent 7514558b

logd/FlushCommand.h

+1 −1
Original line number Diff line number Diff line
@@ -16,7 +16,7 @@ 
#ifndef _FLUSH_COMMAND_H

#define _FLUSH_COMMAND_H



#include <private/android_logger.h>

#include <android/log.h>

#include <sysutils/SocketClientCommand.h>



class LogBufferElement;

logd/LogBuffer.cpp

+16 −26
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ 
#include <unistd.h>



#include <unordered_map>

#include <utility>



#include <cutils/properties.h>

#include <private/android_logger.h>
@@ -560,12 +561,11 @@ class LogBufferElementLast { 
    }



    void clear(LogBufferElement* element) {

        log_time current =

            element->getRealTime() - log_time(EXPIRE_RATELIMIT, 0);

        uint64_t current = element->getRealTime().nsec() - (EXPIRE_RATELIMIT * NS_PER_SEC);

        for (LogBufferElementMap::iterator it = map.begin(); it != map.end();) {

            LogBufferElement* mapElement = it->second;

            if ((mapElement->getDropped() >= EXPIRE_THRESHOLD) &&

                (current > mapElement->getRealTime())) {

            if (mapElement->getDropped() >= EXPIRE_THRESHOLD &&

                current > mapElement->getRealTime().nsec()) {

                it = map.erase(it);

            } else {

                ++it;
@@ -683,7 +683,7 @@ bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) { 
                mLastSet[id] = true;

            }



            if (oldest && (oldest->mStart <= element->getRealTime().nsec())) {

            if (oldest && oldest->mStart <= element->getSequence()) {

                busy = true;

                kickMe(oldest, id, pruneRows);

                break;
@@ -771,7 +771,7 @@ bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) { 
        while (it != mLogElements.end()) {

            LogBufferElement* element = *it;



            if (oldest && (oldest->mStart <= element->getRealTime().nsec())) {

            if (oldest && oldest->mStart <= element->getSequence()) {

                busy = true;

                // Do not let chatty eliding trigger any reader mitigation

                break;
@@ -923,7 +923,7 @@ bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) { 
            mLastSet[id] = true;

        }



        if (oldest && (oldest->mStart <= element->getRealTime().nsec())) {

        if (oldest && oldest->mStart <= element->getSequence()) {

            busy = true;

            if (!whitelist) kickMe(oldest, id, pruneRows);

            break;
@@ -956,7 +956,7 @@ bool LogBuffer::prune(log_id_t id, unsigned long pruneRows, uid_t caller_uid) { 
                mLastSet[id] = true;

            }



            if (oldest && (oldest->mStart <= element->getRealTime().nsec())) {

            if (oldest && oldest->mStart <= element->getSequence()) {

                busy = true;

                kickMe(oldest, id, pruneRows);

                break;
@@ -1045,42 +1045,32 @@ unsigned long LogBuffer::getSize(log_id_t id) { 
    return retval;

}



log_time LogBuffer::flushTo(SocketClient* reader, const log_time& start,

                            pid_t* lastTid, bool privileged, bool security,

                            int (*filter)(const LogBufferElement* element,

                                          void* arg),

                            void* arg) {

uint64_t LogBuffer::flushTo(SocketClient* reader, uint64_t start, pid_t* lastTid, bool privileged,

                            bool security,

                            int (*filter)(const LogBufferElement* element, void* arg), void* arg) {

    LogBufferElementCollection::iterator it;

    uid_t uid = reader->getUid();



    rdlock();



    if (start == log_time::EPOCH) {

    if (start <= 1) {

        // client wants to start from the beginning

        it = mLogElements.begin();

    } else {

        // Cap to 300 iterations we look back for out-of-order entries.

        size_t count = 300;

        // Client wants to start from some specified time. Chances are

        // we are better off starting from the end of the time sorted list.

        LogBufferElementCollection::iterator last;

        for (last = it = mLogElements.end(); it != mLogElements.begin();

        for (it = mLogElements.end(); it != mLogElements.begin();

             /* do nothing */) {

            --it;

            LogBufferElement* element = *it;

            if (element->getRealTime() > start) {

                last = it;

            } else if (element->getRealTime() == start) {

                last = ++it;

                break;

            } else if (!--count) {

            if (element->getSequence() <= start) {

                it++;

                break;

            }

        }

        it = last;

    }



    log_time curr = start;

    uint64_t curr = start;



    LogBufferElement* lastElement = nullptr;  // iterator corruption paranoia

    static const size_t maxSkip = 4194304;    // maximum entries to skip

logd/LogBuffer.h

+2 −3
Original line number Diff line number Diff line
@@ -118,11 +118,10 @@ class LogBuffer { 
    // lastTid is an optional context to help detect if the last previous

    // valid message was from the same source so we can differentiate chatty

    // filter types (identical or expired)

    log_time flushTo(SocketClient* writer, const log_time& start,

    uint64_t flushTo(SocketClient* writer, uint64_t start,

                     pid_t* lastTid,  // &lastTid[LOG_ID_MAX] or nullptr

                     bool privileged, bool security,

                     int (*filter)(const LogBufferElement* element,

                                   void* arg) = nullptr,

                     int (*filter)(const LogBufferElement* element, void* arg) = nullptr,

                     void* arg = nullptr);



    bool clear(log_id_t id, uid_t uid = AID_ROOT);

logd/LogBufferElement.cpp

+8 −9
Original line number Diff line number Diff line
@@ -30,15 +30,15 @@ 
#include "LogReader.h"

#include "LogUtils.h"



const log_time LogBufferElement::FLUSH_ERROR((uint32_t)-1, (uint32_t)-1);

const uint64_t LogBufferElement::FLUSH_ERROR(0);

atomic_int_fast64_t LogBufferElement::sequence(1);



LogBufferElement::LogBufferElement(log_id_t log_id, log_time realtime,

                                   uid_t uid, pid_t pid, pid_t tid,

                                   const char* msg, uint16_t len)

LogBufferElement::LogBufferElement(log_id_t log_id, log_time realtime, uid_t uid, pid_t pid,

                                   pid_t tid, const char* msg, uint16_t len)

    : mUid(uid),

      mPid(pid),

      mTid(tid),

      mSequence(sequence.fetch_add(1, memory_order_relaxed)),

      mRealTime(realtime),

      mMsgLen(len),

      mLogId(log_id),
@@ -51,6 +51,7 @@ LogBufferElement::LogBufferElement(const LogBufferElement& elem) 
    : mUid(elem.mUid),

      mPid(elem.mPid),

      mTid(elem.mTid),

      mSequence(elem.mSequence),

      mRealTime(elem.mRealTime),

      mMsgLen(elem.mMsgLen),

      mLogId(elem.mLogId),
@@ -244,7 +245,7 @@ size_t LogBufferElement::populateDroppedMessage(char*& buffer, LogBuffer* parent 
    return retval;

}



log_time LogBufferElement::flushTo(SocketClient* reader, LogBuffer* parent, bool lastSame) {

uint64_t LogBufferElement::flushTo(SocketClient* reader, LogBuffer* parent, bool lastSame) {

    struct logger_entry entry = {};



    entry.hdr_size = sizeof(struct logger_entry);
@@ -263,7 +264,7 @@ log_time LogBufferElement::flushTo(SocketClient* reader, LogBuffer* parent, bool 


    if (mDropped) {

        entry.len = populateDroppedMessage(buffer, parent, lastSame);

        if (!entry.len) return mRealTime;

        if (!entry.len) return mSequence;

        iovec[1].iov_base = buffer;

    } else {

        entry.len = mMsgLen;
@@ -271,9 +272,7 @@ log_time LogBufferElement::flushTo(SocketClient* reader, LogBuffer* parent, bool 
    }

    iovec[1].iov_len = entry.len;



    log_time retval = reader->sendDatav(iovec, 1 + (entry.len != 0))

                          ? FLUSH_ERROR

                          : mRealTime;

    uint64_t retval = reader->sendDatav(iovec, 1 + (entry.len != 0)) ? FLUSH_ERROR : mSequence;



    if (buffer) free(buffer);

logd/LogBufferElement.h

+5 −2
Original line number Diff line number Diff line
@@ -39,6 +39,7 @@ class __attribute__((packed)) LogBufferElement { 
    const uint32_t mUid;

    const uint32_t mPid;

    const uint32_t mTid;

    uint64_t mSequence;

    log_time mRealTime;

    union {

        char* mMsg;    // mDropped == false
@@ -90,10 +91,12 @@ class __attribute__((packed)) LogBufferElement { 
    const char* getMsg() const {

        return mDropped ? nullptr : mMsg;

    }

    uint64_t getSequence() const { return mSequence; }

    static uint64_t getCurrentSequence() { return sequence.load(memory_order_relaxed); }

    log_time getRealTime(void) const {

        return mRealTime;

    }



    static const log_time FLUSH_ERROR;

    log_time flushTo(SocketClient* writer, LogBuffer* parent, bool lastSame);

    static const uint64_t FLUSH_ERROR;

    uint64_t flushTo(SocketClient* writer, LogBuffer* parent, bool lastSame);

};