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

Commit d33928b3 authored by Hawkwood's avatar Hawkwood
Browse files

Fix threading problem with LogcatOnlyMessageBuffer 

LogcatOnlyMessageBuffer was originally intended to only use one message
by having it be committed (and freed) immediately after it was obtained.
This order was guarenteed by synchronizing them, however two potential
issues are present with that approach.

 - A second thread could call obtain before the first thread called
   commit. This would cause the second thread to throw an exception.
 - The `messagePrinter` callback could throw an exception while creating
   the result string. This leaves the object in a unrecoverable state.

This change solves both by creating a small circular list of messages to
use, which has a number of benefits.

 - Messages can be seamlessly lost or fail and will be recreated without
   impacting long-term object state.
 - A small max queue size can be enforced to limit baseline memory use
 - Additional temporary messages can be easily created when needed
 - Most log calls will be able to reuse an existing object from the pool
   instead of incurring an allocation.
 - Only operations on the message queue need to be synchronized as all
   other operations are executed on distinct objects.

Bug: 414815071
Test: Checked logging functions
Flag: NONE Bugfix
Change-Id: I12445ed27f0336b14608f3f46b6e11077f87b1a6
parent 91380375

packages/SystemUI/log/src/com/android/systemui/log/LogMessageImpl.kt

+5 −1
Original line number Diff line number Diff line
@@ -41,6 +41,10 @@ data class LogMessageImpl( 
    override var bool4: Boolean,

) : LogMessage {



    fun clear() {

        reset(DEFAULT_TAG, LogLevel.DEBUG, 0, DEFAULT_PRINTER)

    }



    fun reset(

        tag: String,

        level: LogLevel,
@@ -86,7 +90,7 @@ data class LogMessageImpl( 
                false,

                false,

                false,

                false

                false,

            )

        }

    }

packages/SystemUI/log/src/com/android/systemui/log/core/LogcatOnlyMessageBuffer.kt

+20 −23
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ package com.android.systemui.log.core 


import android.util.Log

import com.android.systemui.log.LogMessageImpl

import kotlin.collections.ArrayDeque



/**

 * A simple implementation of [MessageBuffer] that forwards messages to [android.util.Log]
@@ -26,39 +27,24 @@ import com.android.systemui.log.LogMessageImpl 
 */

class LogcatOnlyMessageBuffer(

    val targetLogLevel: LogLevel,

    val maxMessageCount: Int = DEFAULT_MESSAGE_MAX_COUNT,

) : MessageBuffer {

    private val singleMessage = LogMessageImpl.Factory.create()

    private var isObtained: Boolean = false

    private val messages = ArrayDeque<LogMessageImpl>(maxMessageCount)



    @Synchronized

    override fun obtain(

        tag: String,

        level: LogLevel,

        messagePrinter: MessagePrinter,

        exception: Throwable?,

    ): LogMessage {

        if (isObtained) {

            throw UnsupportedOperationException(

                "Message has already been obtained. Call order is incorrect."

            )

        val message =

            synchronized(messages) { messages.removeFirstOrNull() }

                ?: LogMessageImpl.Factory.create()

        message.reset(tag, level, System.currentTimeMillis(), messagePrinter, exception)

        return message

    }



        singleMessage.reset(tag, level, System.currentTimeMillis(), messagePrinter, exception)

        isObtained = true

        return singleMessage

    }



    @Synchronized

    override fun commit(message: LogMessage) {

        if (singleMessage != message) {

            throw IllegalArgumentException("Message argument is not the expected message.")

        }

        if (!isObtained) {

            throw UnsupportedOperationException(

                "Message has not been obtained. Call order is incorrect."

            )

        }



        if (message.level >= targetLogLevel) {

            val strMessage = message.messagePrinter(message)

            when (message.level) {
@@ -71,6 +57,17 @@ class LogcatOnlyMessageBuffer( 
            }

        }



        isObtained = false

        if (message is LogMessageImpl) {

            message.clear()

            synchronized(messages) {

                if (messages.size < maxMessageCount) {

                    messages.addLast(message)

                }

            }

        }

    }



    companion object {

        val DEFAULT_MESSAGE_MAX_COUNT = 4

    }

}