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

Commit 286f4f9e authored by Vishnu Nair's avatar Vishnu Nair
Browse files

LayerTraceGenerator: Fix duplicate layer ids 

Layers can be created from API calls on the binder thread or
by surfaceflinger on the main thread for mirroring and
BSL background layers. Transaction traces keep track of
layer creation API calls to recreate layers when generating
the layer trace.

However, there was a possibility that new layers would be
recorded with a set of transactions that were applied with
an earlier vsync. This is harmless except we could end up
creating layers with duplicate layer ids since layers created
via binder would get an injected sequence id while main
thread created layers would use the global counter.

Test: atest transactiontrace_testsuite
Bug: 235376060
Change-Id: Ia11839d1880bc131ab5314779281c93393d6742f
parent e3066de6

services/surfaceflinger/SurfaceFlinger.cpp

+12 −10
Original line number Diff line number Diff line
@@ -2056,6 +2056,11 @@ bool SurfaceFlinger::commit(nsecs_t frameTime, int64_t vsyncId, nsecs_t expected 
        mGpuFrameMissedCount++;

    }



    if (mTracingEnabledChanged) {

        mLayerTracingEnabled = mLayerTracing.isEnabled();

        mTracingEnabledChanged = false;

    }



    // If we are in the middle of a mode change and the fence hasn't

    // fired yet just wait for the next commit.

    if (mSetActiveModePending) {
@@ -2104,11 +2109,6 @@ bool SurfaceFlinger::commit(nsecs_t frameTime, int64_t vsyncId, nsecs_t expected 
        }

    }



    if (mTracingEnabledChanged) {

        mLayerTracingEnabled = mLayerTracing.isEnabled();

        mTracingEnabledChanged = false;

    }



    if (mRefreshRateOverlaySpinner) {

        Mutex::Autolock lock(mStateLock);

        if (const auto display = getDefaultDisplayDeviceLocked()) {
@@ -2123,7 +2123,7 @@ bool SurfaceFlinger::commit(nsecs_t frameTime, int64_t vsyncId, nsecs_t expected 


        bool needsTraversal = false;

        if (clearTransactionFlags(eTransactionFlushNeeded)) {

            needsTraversal |= commitCreatedLayers();

            needsTraversal |= commitCreatedLayers(vsyncId);

            needsTraversal |= flushTransactionQueues(vsyncId);

        }
@@ -7154,7 +7154,7 @@ int SurfaceFlinger::getMaxAcquiredBufferCountForRefreshRate(Fps refreshRate) con 
    return calculateMaxAcquiredBufferCount(refreshRate, presentLatency);

}



void SurfaceFlinger::handleLayerCreatedLocked(const LayerCreatedState& state) {

void SurfaceFlinger::handleLayerCreatedLocked(const LayerCreatedState& state, int64_t vsyncId) {

    sp<Layer> layer = state.layer.promote();

    if (!layer) {

        ALOGD("Layer was destroyed soon after creation %p", state.layer.unsafe_get());
@@ -7184,7 +7184,9 @@ void SurfaceFlinger::handleLayerCreatedLocked(const LayerCreatedState& state) { 
    }



    layer->updateTransformHint(mActiveDisplayTransformHint);



    if (mTransactionTracing) {

        mTransactionTracing->onLayerAddedToDrawingState(layer->getSequence(), vsyncId);

    }

    mInterceptor->saveSurfaceCreation(layer);

}
@@ -7268,7 +7270,7 @@ std::shared_ptr<renderengine::ExternalTexture> SurfaceFlinger::getExternalTextur 
    return buffer;

}



bool SurfaceFlinger::commitCreatedLayers() {

bool SurfaceFlinger::commitCreatedLayers(int64_t vsyncId) {

    std::vector<LayerCreatedState> createdLayers;

    {

        std::scoped_lock<std::mutex> lock(mCreatedLayersLock);
@@ -7281,7 +7283,7 @@ bool SurfaceFlinger::commitCreatedLayers() { 


    Mutex::Autolock _l(mStateLock);

    for (const auto& createdLayer : createdLayers) {

        handleLayerCreatedLocked(createdLayer);

        handleLayerCreatedLocked(createdLayer, vsyncId);

    }

    createdLayers.clear();

    mLayersAdded = true;

services/surfaceflinger/SurfaceFlinger.h

+3 −2
Original line number Diff line number Diff line
@@ -1405,8 +1405,9 @@ private: 
    // A temporay pool that store the created layers and will be added to current state in main

    // thread.

    std::vector<LayerCreatedState> mCreatedLayers GUARDED_BY(mCreatedLayersLock);

    bool commitCreatedLayers();

    void handleLayerCreatedLocked(const LayerCreatedState& state) REQUIRES(mStateLock);

    bool commitCreatedLayers(int64_t vsyncId);

    void handleLayerCreatedLocked(const LayerCreatedState& state, int64_t vsyncId)

            REQUIRES(mStateLock);



    std::atomic<ui::Transform::RotationFlags> mActiveDisplayTransformHint;

services/surfaceflinger/Tracing/TransactionProtoParser.cpp

+7 −2
Original line number Diff line number Diff line
@@ -498,8 +498,13 @@ void TransactionProtoParser::fromProto(const proto::LayerState& proto, layer_sta 
        inputInfo.replaceTouchableRegionWithCrop =

                windowInfoProto.replace_touchable_region_with_crop();

        int64_t layerId = windowInfoProto.crop_layer_id();

        if (layerId != -1) {

            inputInfo.touchableRegionCropHandle =

                    mMapper->getLayerHandle(static_cast<int32_t>(layerId));

        } else {

            inputInfo.touchableRegionCropHandle = nullptr;

        }



        layer.windowInfoHandle = sp<gui::WindowInfoHandle>::make(inputInfo);

    }

    if (proto.what() & layer_state_t::eBackgroundColorChanged) {

services/surfaceflinger/Tracing/TransactionTracing.cpp

+35 −10
Original line number Diff line number Diff line
@@ -152,17 +152,33 @@ void TransactionTracing::addQueuedTransaction(const TransactionState& transactio 
    mTransactionQueue.push(state);

}



void TransactionTracing::addCommittedTransactions(std::vector<TransactionState>& transactions,

                                                  int64_t vsyncId) {

TransactionTracing::CommittedTransactions&

TransactionTracing::findOrCreateCommittedTransactionRecord(int64_t vsyncId) {

    for (auto& pendingTransaction : mPendingTransactions) {

        if (pendingTransaction.vsyncId == vsyncId) {

            return pendingTransaction;

        }

    }



    CommittedTransactions committedTransactions;

    committedTransactions.vsyncId = vsyncId;

    committedTransactions.timestamp = systemTime();

    mPendingTransactions.emplace_back(committedTransactions);

    return mPendingTransactions.back();

}



void TransactionTracing::onLayerAddedToDrawingState(int layerId, int64_t vsyncId) {

    CommittedTransactions& committedTransactions = findOrCreateCommittedTransactionRecord(vsyncId);

    committedTransactions.createdLayerIds.emplace_back(layerId);

}



void TransactionTracing::addCommittedTransactions(std::vector<TransactionState>& transactions,

                                                  int64_t vsyncId) {

    CommittedTransactions& committedTransactions = findOrCreateCommittedTransactionRecord(vsyncId);

    committedTransactions.transactionIds.reserve(transactions.size());

    for (const auto& transaction : transactions) {

        committedTransactions.transactionIds.emplace_back(transaction.id);

    }



    mPendingTransactions.emplace_back(committedTransactions);

    tryPushToTracingThread();

}
@@ -235,15 +251,24 @@ void TransactionTracing::addEntry(const std::vector<CommittedTransactions>& comm 
    for (const CommittedTransactions& entry : committedTransactions) {

        entryProto.set_elapsed_realtime_nanos(entry.timestamp);

        entryProto.set_vsync_id(entry.vsyncId);

        entryProto.mutable_added_layers()->Reserve(static_cast<int32_t>(mCreatedLayers.size()));

        for (auto& newLayer : mCreatedLayers) {

            entryProto.mutable_added_layers()->Add(std::move(newLayer));

        entryProto.mutable_added_layers()->Reserve(

                static_cast<int32_t>(entry.createdLayerIds.size()));



        for (const int32_t& id : entry.createdLayerIds) {

            auto it = mCreatedLayers.find(id);

            if (it != mCreatedLayers.end()) {

                entryProto.mutable_added_layers()->Add(std::move(it->second));

                mCreatedLayers.erase(it);

            } else {

                ALOGW("Could not created layer with id %d", id);

            }

        }



        entryProto.mutable_removed_layers()->Reserve(static_cast<int32_t>(removedLayers.size()));

        for (auto& removedLayer : removedLayers) {

            entryProto.mutable_removed_layers()->Add(removedLayer);

            mCreatedLayers.erase(removedLayer);

        }

        mCreatedLayers.clear();

        entryProto.mutable_transactions()->Reserve(

                static_cast<int32_t>(entry.transactionIds.size()));

        for (const uint64_t& id : entry.transactionIds) {
@@ -304,7 +329,7 @@ void TransactionTracing::onLayerAdded(BBinder* layerHandle, int layerId, const s 
        ALOGW("Duplicate handles found. %p", layerHandle);

    }

    mLayerHandles[layerHandle] = layerId;

    mCreatedLayers.push_back(mProtoParser.toProto(args));

    mCreatedLayers[layerId] = mProtoParser.toProto(args);

}



void TransactionTracing::onMirrorLayerAdded(BBinder* layerHandle, int layerId,
@@ -315,7 +340,7 @@ void TransactionTracing::onMirrorLayerAdded(BBinder* layerHandle, int layerId, 
        ALOGW("Duplicate handles found. %p", layerHandle);

    }

    mLayerHandles[layerHandle] = layerId;

    mCreatedLayers.emplace_back(mProtoParser.toProto(args));

    mCreatedLayers[layerId] = mProtoParser.toProto(args);

}



void TransactionTracing::onLayerRemoved(int32_t layerId) {

services/surfaceflinger/Tracing/TransactionTracing.h

+4 −2
Original line number Diff line number Diff line
@@ -64,6 +64,7 @@ public: 
                            int mirrorFromId);

    void onLayerRemoved(int layerId);

    void onHandleRemoved(BBinder* layerHandle);

    void onLayerAddedToDrawingState(int layerId, int64_t vsyncId);

    void dump(std::string&) const;

    static constexpr auto CONTINUOUS_TRACING_BUFFER_SIZE = 512 * 1024;

    static constexpr auto ACTIVE_TRACING_BUFFER_SIZE = 100 * 1024 * 1024;
@@ -81,7 +82,7 @@ private: 
            GUARDED_BY(mTraceLock);

    LocklessStack<proto::TransactionState> mTransactionQueue;

    nsecs_t mStartingTimestamp GUARDED_BY(mTraceLock);

    std::vector<proto::LayerCreationArgs> mCreatedLayers GUARDED_BY(mTraceLock);

    std::unordered_map<int, proto::LayerCreationArgs> mCreatedLayers GUARDED_BY(mTraceLock);

    std::unordered_map<BBinder* /* layerHandle */, int32_t /* layerId */> mLayerHandles

            GUARDED_BY(mTraceLock);

    std::vector<int32_t /* layerId */> mRemovedLayerHandles GUARDED_BY(mTraceLock);
@@ -100,6 +101,7 @@ private: 
    std::condition_variable mTransactionsAddedToBufferCv;

    struct CommittedTransactions {

        std::vector<uint64_t> transactionIds;

        std::vector<int32_t> createdLayerIds;

        int64_t vsyncId;

        int64_t timestamp;

    };
@@ -117,7 +119,7 @@ private: 
    void tryPushToTracingThread() EXCLUDES(mMainThreadLock);

    void addStartingStateToProtoLocked(proto::TransactionTraceFile& proto) REQUIRES(mTraceLock);

    void updateStartingStateLocked(const proto::TransactionTraceEntry& entry) REQUIRES(mTraceLock);



    CommittedTransactions& findOrCreateCommittedTransactionRecord(int64_t vsyncId);

    // TEST

    // Wait until all the committed transactions for the specified vsync id are added to the buffer.

    void flush(int64_t vsyncId) EXCLUDES(mMainThreadLock);