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

Commit 1fdbdbe1 authored by Tom Cherry's avatar Tom Cherry
Browse files

logd: join() the SerializedLogBuffer deleter thread 

Logd never deletes SerializedLogBuffer, so it seemed reasonable to
detach the deleter thread, however unit tests and fuzzers do delete
SerializedLogBuffer, so we must safely join the deleter thread in the
destructor.

This simplifies the deleter thread code and ensures that only one
deleter thread will be running at a time.

Test: fuzzing works
Change-Id: I69c7447109898a1bb7038a03337cadacb1213281
parent a2f97dbb

logd/SerializedLogBuffer.cpp

+47 −17
Original line number Diff line number Diff line
@@ -16,8 +16,9 @@ 


#include "SerializedLogBuffer.h"



#include <sys/prctl.h>



#include <limits>

#include <thread>



#include <android-base/logging.h>

#include <android-base/scopeguard.h>
@@ -31,7 +32,11 @@ SerializedLogBuffer::SerializedLogBuffer(LogReaderList* reader_list, LogTags* ta 
    Init();

}



SerializedLogBuffer::~SerializedLogBuffer() {}

SerializedLogBuffer::~SerializedLogBuffer() {

    if (deleter_thread_.joinable()) {

        deleter_thread_.join();

    }

}



void SerializedLogBuffer::Init() {

    log_id_for_each(i) {
@@ -119,15 +124,44 @@ void SerializedLogBuffer::MaybePrune(log_id_t log_id) { 
    }

}



void SerializedLogBuffer::StartDeleterThread() {

    if (deleter_thread_running_) {

        return;

    }

    if (deleter_thread_.joinable()) {

        deleter_thread_.join();

    }

    auto new_thread = std::thread([this] { DeleterThread(); });

    deleter_thread_.swap(new_thread);

    deleter_thread_running_ = true;

}



// Decompresses the chunks, call LogStatistics::Subtract() on each entry, then delete the chunks and

// the list.  Note that the SerializedLogChunk objects have been removed from logs_ and their

// references have been deleted from any SerializedFlushToState objects, so this can be safely done

// without holding lock_.  It is done in a separate thread to avoid delaying the writer thread.  The

// lambda for the thread takes ownership of the 'chunks' list and thus when the thread ends and the

// lambda is deleted, the objects are deleted.

void SerializedLogBuffer::DeleteLogChunks(std::list<SerializedLogChunk>&& chunks, log_id_t log_id) {

    auto delete_thread = std::thread{[chunks = std::move(chunks), log_id, this]() mutable {

        for (auto& chunk : chunks) {

// without holding lock_.  It is done in a separate thread to avoid delaying the writer thread.

void SerializedLogBuffer::DeleterThread() {

    prctl(PR_SET_NAME, "logd.deleter");

    while (true) {

        std::list<SerializedLogChunk> local_chunks_to_delete;

        log_id_t log_id;

        {

            auto lock = std::lock_guard{lock_};

            log_id_for_each(i) {

                if (!chunks_to_delete_[i].empty()) {

                    local_chunks_to_delete = std::move(chunks_to_delete_[i]);

                    chunks_to_delete_[i].clear();

                    log_id = i;

                    break;

                }

            }

            if (local_chunks_to_delete.empty()) {

                deleter_thread_running_ = false;

                return;

            }

        }



        for (auto& chunk : local_chunks_to_delete) {

            chunk.IncReaderRefCount();

            int read_offset = 0;

            while (read_offset < chunk.write_offset()) {
@@ -137,8 +171,7 @@ void SerializedLogBuffer::DeleteLogChunks(std::list<SerializedLogChunk>&& chunks 
            }

            chunk.DecReaderRefCount(false);

        }

    }};

    delete_thread.detach();

    }

}



void SerializedLogBuffer::NotifyReadersOfPrune(
@@ -164,13 +197,9 @@ bool SerializedLogBuffer::Prune(log_id_t log_id, size_t bytes_to_free, uid_t uid 
        }

    }



    auto& log_buffer = logs_[log_id];



    std::list<SerializedLogChunk> chunks_to_prune;

    auto prune_chunks = android::base::make_scope_guard([&chunks_to_prune, log_id, this] {

        DeleteLogChunks(std::move(chunks_to_prune), log_id);

    });

    StartDeleterThread();



    auto& log_buffer = logs_[log_id];

    auto it = log_buffer.begin();

    while (it != log_buffer.end()) {

        if (oldest != nullptr && it->highest_sequence_number() >= oldest->start()) {
@@ -193,7 +222,8 @@ bool SerializedLogBuffer::Prune(log_id_t log_id, size_t bytes_to_free, uid_t uid 
            }

        } else {

            size_t buffer_size = it_to_prune->PruneSize();

            chunks_to_prune.splice(chunks_to_prune.end(), log_buffer, it_to_prune);

            chunks_to_delete_[log_id].splice(chunks_to_delete_[log_id].end(), log_buffer,

                                             it_to_prune);

            if (buffer_size >= bytes_to_free) {

                return true;

            }

logd/SerializedLogBuffer.h

+8 −1
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ 
#include <list>

#include <mutex>

#include <queue>

#include <thread>

#include <vector>



#include <android-base/thread_annotations.h>
@@ -60,7 +61,9 @@ class SerializedLogBuffer final : public LogBuffer { 
            REQUIRES_SHARED(lock_);

    void NotifyReadersOfPrune(log_id_t log_id, const std::list<SerializedLogChunk>::iterator& chunk)

            REQUIRES(reader_list_->reader_threads_lock());

    void DeleteLogChunks(std::list<SerializedLogChunk>&& chunks, log_id_t log_id);



    void StartDeleterThread() REQUIRES(lock_);

    void DeleterThread();



    LogReaderList* reader_list_;

    LogTags* tags_;
@@ -70,5 +73,9 @@ class SerializedLogBuffer final : public LogBuffer { 
    std::list<SerializedLogChunk> logs_[LOG_ID_MAX] GUARDED_BY(lock_);

    RwLock lock_;



    std::list<SerializedLogChunk> chunks_to_delete_[LOG_ID_MAX] GUARDED_BY(lock_);

    std::thread deleter_thread_ GUARDED_BY(lock_);

    bool deleter_thread_running_ GUARDED_BY(lock_) = false;



    std::atomic<uint64_t> sequence_ = 1;

};