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

Commit 11f02d7e authored by Andres Morales's avatar Andres Morales
Browse files

allow for slow FrameMetricsListeners

A slow listener could cause a race in the NotifyHandler
where the single reference to the buffer to send would get
updated when it shouldn't have been.

Switch to a queue of available buffers to prevent this race.

Also, stop setting and clearing the observer reference and instead
incStrong/decStrong to mark temporary strong ownership without
colliding with other owners in flight.

Bug: 27097094
Change-Id: Iee647bfae8b80019b6d8290179eed3973230901f
parent af9aa07f
Loading
Loading
Loading
Loading
+61 −37
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@
#define LOG_TAG "ThreadedRenderer"

#include <algorithm>
#include <atomic>

#include "jni.h"
#include <nativehelper/JNIHelp.h>
@@ -231,28 +232,13 @@ class ObserverProxy;

class NotifyHandler : public MessageHandler {
public:
    NotifyHandler(JavaVM* vm) : mVm(vm) {}

    void setObserver(ObserverProxy* observer) {
        mObserver = observer;
    }

    void setBuffer(BufferPool::Buffer* buffer) {
        mBuffer = buffer;
    }

    void setDropCount(int dropCount) {
        mDropCount = dropCount;
    }
    NotifyHandler(JavaVM* vm, ObserverProxy* observer) : mVm(vm), mObserver(observer) {}

    virtual void handleMessage(const Message& message);

private:
    JavaVM* mVm;

    sp<ObserverProxy> mObserver;
    BufferPool::Buffer* mBuffer = nullptr;
    int mDropCount = 0;
    JavaVM* const mVm;
    ObserverProxy* const mObserver;
};

static jlongArray get_metrics_buffer(JNIEnv* env, jobject observer) {
@@ -265,6 +251,9 @@ static jlongArray get_metrics_buffer(JNIEnv* env, jobject observer) {
    return reinterpret_cast<jlongArray>(buffer);
}

/*
 * Implements JNI layer for hwui frame metrics reporting.
 */
class ObserverProxy : public FrameMetricsObserver {
public:
    ObserverProxy(JavaVM *vm, jobject observer) : mVm(vm) {
@@ -284,7 +273,7 @@ public:
        mMessageQueue = android_os_MessageQueue_getMessageQueue(env, messageQueueLocal);
        LOG_ALWAYS_FATAL_IF(mMessageQueue == nullptr, "message queue not available");

        mMessageHandler = new NotifyHandler(mVm);
        mMessageHandler = new NotifyHandler(mVm, this);
        LOG_ALWAYS_FATAL_IF(mMessageHandler == nullptr,
                "OOM: unable to allocate NotifyHandler");
    }
@@ -298,18 +287,53 @@ public:
        return mObserverWeak;
    }

    virtual void notify(BufferPool::Buffer* buffer, int dropCount) {
        buffer->incRef();
        mMessageHandler->setBuffer(buffer);
        mMessageHandler->setObserver(this);
        mMessageHandler->setDropCount(dropCount);
    bool getNextBuffer(JNIEnv* env, jlongArray sink, int* dropCount) {
        FrameMetricsNotification& elem = mRingBuffer[mNextInQueue];

        if (elem.hasData.load()) {
            env->SetLongArrayRegion(sink, 0, kBufferSize, elem.buffer);
            *dropCount = elem.dropCount;
            mNextInQueue = (mNextInQueue + 1) % kRingSize;
            elem.hasData = false;
            return true;
        }

        return false;
    }

    virtual void notify(const int64_t* stats) {
        FrameMetricsNotification& elem = mRingBuffer[mNextFree];

        if (!elem.hasData.load()) {
            memcpy(elem.buffer, stats, kBufferSize * sizeof(stats[0]));

            elem.dropCount = mDroppedReports;
            mDroppedReports = 0;

            incStrong(nullptr);
            mNextFree = (mNextFree + 1) % kRingSize;
            elem.hasData = true;

            mMessageQueue->getLooper()->sendMessage(mMessageHandler, mMessage);
        } else {
            mDroppedReports++;
        }
    }

private:
    static const int kBufferSize = static_cast<int>(FrameInfoIndex::NumIndexes);
    static constexpr int kRingSize = 3;

    JavaVM* mVm;
    class FrameMetricsNotification {
    public:
        FrameMetricsNotification() : hasData(false) {}

        std::atomic_bool hasData;
        int64_t buffer[kBufferSize];
        int dropCount = 0;
    };

    JavaVM* const mVm;
    jweak mObserverWeak;
    jobject mJavaBufferGlobal;

@@ -317,28 +341,28 @@ private:
    sp<NotifyHandler> mMessageHandler;
    Message mMessage;

    int mNextFree = 0;
    int mNextInQueue = 0;
    FrameMetricsNotification mRingBuffer[kRingSize];

    int mDroppedReports = 0;
};

void NotifyHandler::handleMessage(const Message& message) {
    JNIEnv* env = getenv(mVm);

    ObserverProxy* observer = mObserver.get();
    LOG_ALWAYS_FATAL_IF(observer == nullptr, "received message with no observer configured");
    LOG_ALWAYS_FATAL_IF(mBuffer == nullptr, "received message with no data to report");

    jobject target = env->NewLocalRef(observer->getObserverReference());
    jobject target = env->NewLocalRef(mObserver->getObserverReference());

    if (target != nullptr) {
        jlongArray javaBuffer = get_metrics_buffer(env, target);
        env->SetLongArrayRegion(javaBuffer,
                0, mBuffer->getSize(), mBuffer->getBuffer());
        env->CallVoidMethod(target, gFrameMetricsObserverClassInfo.callback,
                mDropCount);
        int dropCount = 0;
        while (mObserver->getNextBuffer(env, javaBuffer, &dropCount)) {
            env->CallVoidMethod(target, gFrameMetricsObserverClassInfo.callback, dropCount);
        }
        env->DeleteLocalRef(target);
    }

    mBuffer->release();
    mObserver.clear();
    mObserver->decStrong(nullptr);
}

static void android_view_ThreadedRenderer_setAtlas(JNIEnv* env, jobject clazz,
+0 −1
Original line number Diff line number Diff line
@@ -232,7 +232,6 @@ LOCAL_CFLAGS := \

LOCAL_SRC_FILES += \
    $(hwui_test_common_src_files) \
    tests/unit/BufferPoolTests.cpp \
    tests/unit/CanvasStateTests.cpp \
    tests/unit/ClipAreaTests.cpp \
    tests/unit/CrashHandlerInjector.cpp \

libs/hwui/BufferPool.h

deleted100644 → 0
+0 −181
Original line number Diff line number Diff line
/*
 * Copyright (C) 2016 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

#pragma once

#include "utils/RefBase.h"
#include "utils/Log.h"
#include "utils/Macros.h"

#include <atomic>
#include <stdint.h>
#include <memory>
#include <mutex>

namespace android {
namespace uirenderer {

/*
 * Simple thread-safe pool of int64_t arrays of a provided size.
 *
 * Permits allocating a client-provided max number of buffers.
 * If all buffers are in use, refuses to service any more
 * acquire requests until buffers are re-released to the pool.
 */
class BufferPool : public VirtualLightRefBase {
public:
    class Buffer {
        PREVENT_COPY_AND_ASSIGN(Buffer);
    public:
        int64_t* getBuffer() { return mBuffer.get(); }
        size_t getSize() { return mSize; }

        void release() {
            LOG_ALWAYS_FATAL_IF(mPool.get() == nullptr, "attempt to release unacquired buffer");
            mPool->release(this);
        }

        Buffer* incRef() {
            mRefs++;
            return this;
        }

        int decRef() {
            int refs = mRefs.fetch_sub(1);
            LOG_ALWAYS_FATAL_IF(refs == 0, "buffer reference decremented below 0");
            return refs - 1;
        }

        bool isUniqueRef() {
            return mRefs.load() == 1;
        }

    private:
        friend class BufferPool;

        Buffer(BufferPool* pool, size_t size) : mRefs(1) {
            mSize = size;
            mBuffer.reset(new int64_t[size]);
            mPool = pool;
        }

        void setPool(BufferPool* pool) {
            mPool = pool;
        }

        std::unique_ptr<Buffer> mNext;
        std::unique_ptr<int64_t[]> mBuffer;
        sp<BufferPool> mPool;
        size_t mSize;

        std::atomic_int mRefs;
    };

    BufferPool(size_t bufferSize, size_t count)
            : mBufferSize(bufferSize), mCount(count) {}

    /**
     * Acquires a buffer from the buffer pool if available.
     *
     * Only `mCount` buffers are allowed to be in use at a single
     * instance.
     *
     * If no buffer is available, i.e. `mCount` buffers are in use,
     * returns nullptr.
     *
     * The pointer returned from this method *MUST NOT* be freed, instead
     * BufferPool::release() must be called upon it when the client
     * is done with it. Failing to release buffers will eventually make the
     * BufferPool refuse to service any more BufferPool::acquire() requests.
     */
    BufferPool::Buffer* acquire() {
        std::lock_guard<std::mutex> lock(mLock);

        if (mHead.get() != nullptr) {
            BufferPool::Buffer* res = mHead.release();
            mHead = std::move(res->mNext);
            res->mNext.reset(nullptr);
            res->setPool(this);
            res->incRef();
            return res;
        }

        if (mAllocatedCount < mCount) {
            ++mAllocatedCount;
            return new BufferPool::Buffer(this, mBufferSize);
        }

        return nullptr;
    }

    /**
     * Releases a buffer previously acquired by BufferPool::acquire().
     *
     * The released buffer is not valid after calling this method and
     * attempting to use will result in undefined behavior.
     */
    void release(BufferPool::Buffer* buffer) {
        std::lock_guard<std::mutex> lock(mLock);

        if (buffer->decRef() != 0) {
            return;
        }

        buffer->setPool(nullptr);

        BufferPool::Buffer* list = mHead.get();
        if (list == nullptr) {
            mHead.reset(buffer);
            mHead->mNext.reset(nullptr);
            return;
        }

        while (list->mNext.get() != nullptr) {
            list = list->mNext.get();
        }

        list->mNext.reset(buffer);
    }

    /*
     * Used for testing.
     */
    size_t getAvailableBufferCount() {
        size_t remainingToAllocateCount = mCount - mAllocatedCount;

        BufferPool::Buffer* list = mHead.get();
        if (list == nullptr) return remainingToAllocateCount;

        int count = 1;
        while (list->mNext.get() != nullptr) {
            count++;
            list = list->mNext.get();
        }

        return count + remainingToAllocateCount;
    }

private:
    mutable std::mutex mLock;

    size_t mBufferSize;
    size_t mCount;
    size_t mAllocatedCount = 0;
    std::unique_ptr<BufferPool::Buffer> mHead;
};

}; // namespace uirenderer
}; // namespace android
+1 −3
Original line number Diff line number Diff line
@@ -18,14 +18,12 @@

#include <utils/RefBase.h>

#include "BufferPool.h"

namespace android {
namespace uirenderer {

class FrameMetricsObserver : public VirtualLightRefBase {
public:
    virtual void notify(BufferPool::Buffer* buffer, int dropCount);
    virtual void notify(const int64_t* buffer);
};

}; // namespace uirenderer
+3 −30
Original line number Diff line number Diff line
@@ -19,7 +19,6 @@
#include <utils/RefBase.h>
#include <utils/Log.h>

#include "BufferPool.h"
#include "FrameInfo.h"
#include "FrameMetricsObserver.h"

@@ -31,10 +30,7 @@ namespace uirenderer {

class FrameMetricsReporter {
public:
    FrameMetricsReporter() {
        mBufferPool = new BufferPool(kBufferSize, kBufferCount);
        LOG_ALWAYS_FATAL_IF(mBufferPool.get() == nullptr, "OOM: unable to allocate buffer pool");
    }
    FrameMetricsReporter() {}

    void addObserver(FrameMetricsObserver* observer) {
        mObservers.push_back(observer);
@@ -55,36 +51,13 @@ public:
    }

    void reportFrameMetrics(const int64_t* stats) {
        BufferPool::Buffer* statsBuffer = mBufferPool->acquire();

        if (statsBuffer != nullptr) {
            // copy in frame stats
            memcpy(statsBuffer->getBuffer(), stats, kBufferSize * sizeof(*stats));

            // notify on requested threads
        for (size_t i = 0; i < mObservers.size(); i++) {
                mObservers[i]->notify(statsBuffer, mDroppedReports);
            }

            // drop our reference
            statsBuffer->release();
            mDroppedReports = 0;
        } else {
            mDroppedReports++;
            mObservers[i]->notify(stats);
        }
    }

    int getDroppedReports() { return mDroppedReports; }

private:
    static const size_t kBufferCount = 3;
    static const size_t kBufferSize = static_cast<size_t>(FrameInfoIndex::NumIndexes);

    std::vector< sp<FrameMetricsObserver> > mObservers;

    sp<BufferPool> mBufferPool;

    int mDroppedReports = 0;
};

}; // namespace uirenderer
Loading