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

Commit 8406fd78 authored by Vishnu Nair's avatar Vishnu Nair
Browse files

Fix locking issues with proto dumps 

Proto dumps are generated from:
 - binder threads when generating bugreports or triggering dumpstate
 - main thread when mLayerStats is enabled
 - tracing thread when winscope tracing is enabled.

The binder thread reads current state while the other threads reads drawing state.

The writeToProto function accesses a mix of current and drawing states. mPendingState should
only be accessed with the mStateLock held and the visible regions should be read from the main
or tracing threads. This causes some invalid access issues.

To make the locking requirements clear, this change
1. moves drawing specific data to a new function
2. copies mPendingState so we can dump the copy safely in main thread
3. dumps drawing data from binder threads by posting a message onto the main thread

Bug: 138318680
Test: adb shell dumpsys SurfaceFlinger, winscope

Change-Id: I8bb93e9b9f81faec59585b770eb7ba0fbcd9b51b
parent 61eba0db

services/surfaceflinger/Layer.cpp

+56 −57
Original line number Diff line number Diff line
@@ -153,7 +153,6 @@ void Layer::removeRemoteSyncPoints() { 
    mRemoteSyncPoints.clear();



    {

        Mutex::Autolock pendingStateLock(mPendingStateMutex);

        for (State pendingState : mPendingStates) {

            pendingState.barrierLayer_legacy = nullptr;

        }
@@ -853,6 +852,7 @@ uint32_t Layer::doTransaction(uint32_t flags) { 


    // Commit the transaction

    commitTransaction(c);

    mPendingStatesSnapshot = mPendingStates;

    mCurrentState.callbackHandles = {};

    return flags;

}
@@ -1820,14 +1820,61 @@ void Layer::setInputInfo(const InputWindowInfo& info) { 
    setTransactionFlags(eTransactionNeeded);

}



void Layer::writeToProto(LayerProto* layerInfo, LayerVector::StateSet stateSet,

                         uint32_t traceFlags) {

void Layer::writeToProtoDrawingState(LayerProto* layerInfo, uint32_t traceFlags) const {

    ui::Transform transform = getTransform();



    if (traceFlags & SurfaceTracing::TRACE_CRITICAL) {

        for (const auto& pendingState : mPendingStatesSnapshot) {

            auto barrierLayer = pendingState.barrierLayer_legacy.promote();

            if (barrierLayer != nullptr) {

                BarrierLayerProto* barrierLayerProto = layerInfo->add_barrier_layer();

                barrierLayerProto->set_id(barrierLayer->sequence);

                barrierLayerProto->set_frame_number(pendingState.frameNumber_legacy);

            }

        }



        auto buffer = mActiveBuffer;

        if (buffer != nullptr) {

            LayerProtoHelper::writeToProto(buffer,

                                           [&]() { return layerInfo->mutable_active_buffer(); });

            LayerProtoHelper::writeToProto(ui::Transform(mCurrentTransform),

                                           layerInfo->mutable_buffer_transform());

        }

        layerInfo->set_invalidate(contentDirty);

        layerInfo->set_is_protected(isProtected());

        layerInfo->set_dataspace(

                dataspaceDetails(static_cast<android_dataspace>(mCurrentDataSpace)));

        layerInfo->set_queued_frames(getQueuedFrameCount());

        layerInfo->set_refresh_pending(isBufferLatched());

        layerInfo->set_curr_frame(mCurrentFrameNumber);

        layerInfo->set_effective_scaling_mode(getEffectiveScalingMode());



        layerInfo->set_corner_radius(getRoundedCornerState().radius);

        LayerProtoHelper::writeToProto(transform, layerInfo->mutable_transform());

        LayerProtoHelper::writePositionToProto(transform.tx(), transform.ty(),

                                               [&]() { return layerInfo->mutable_position(); });

        LayerProtoHelper::writeToProto(mBounds, [&]() { return layerInfo->mutable_bounds(); });

        LayerProtoHelper::writeToProto(visibleRegion,

                                       [&]() { return layerInfo->mutable_visible_region(); });

        LayerProtoHelper::writeToProto(surfaceDamageRegion,

                                       [&]() { return layerInfo->mutable_damage_region(); });

    }



    if (traceFlags & SurfaceTracing::TRACE_EXTRA) {

        LayerProtoHelper::writeToProto(mSourceBounds,

                                       [&]() { return layerInfo->mutable_source_bounds(); });

        LayerProtoHelper::writeToProto(mScreenBounds,

                                       [&]() { return layerInfo->mutable_screen_bounds(); });

    }

}



void Layer::writeToProtoCommonState(LayerProto* layerInfo, LayerVector::StateSet stateSet,

                                    uint32_t traceFlags) const {

    const bool useDrawing = stateSet == LayerVector::StateSet::Drawing;

    const LayerVector& children = useDrawing ? mDrawingChildren : mCurrentChildren;

    const State& state = useDrawing ? mDrawingState : mCurrentState;



    ui::Transform requestedTransform = state.active_legacy.transform;

    ui::Transform transform = getTransform();



    if (traceFlags & SurfaceTracing::TRACE_CRITICAL) {

        layerInfo->set_id(sequence);
@@ -1847,17 +1894,10 @@ void Layer::writeToProto(LayerProto* layerInfo, LayerVector::StateSet stateSet, 


        LayerProtoHelper::writeToProto(state.activeTransparentRegion_legacy,

                                       [&]() { return layerInfo->mutable_transparent_region(); });

        LayerProtoHelper::writeToProto(visibleRegion,

                                       [&]() { return layerInfo->mutable_visible_region(); });

        LayerProtoHelper::writeToProto(surfaceDamageRegion,

                                       [&]() { return layerInfo->mutable_damage_region(); });



        layerInfo->set_layer_stack(getLayerStack());

        layerInfo->set_z(state.z);



        LayerProtoHelper::writePositionToProto(transform.tx(), transform.ty(),

                                               [&]() { return layerInfo->mutable_position(); });



        LayerProtoHelper::writePositionToProto(requestedTransform.tx(), requestedTransform.ty(),

                                               [&]() {

                                                   return layerInfo->mutable_requested_position();
@@ -1868,15 +1908,9 @@ void Layer::writeToProto(LayerProto* layerInfo, LayerVector::StateSet stateSet, 


        LayerProtoHelper::writeToProto(state.crop_legacy,

                                       [&]() { return layerInfo->mutable_crop(); });

        layerInfo->set_corner_radius(getRoundedCornerState().radius);



        layerInfo->set_is_opaque(isOpaque(state));

        layerInfo->set_invalidate(contentDirty);

        layerInfo->set_is_protected(isProtected());



        // XXX (b/79210409) mCurrentDataSpace is not protected

        layerInfo->set_dataspace(

                dataspaceDetails(static_cast<android_dataspace>(mCurrentDataSpace)));



        layerInfo->set_pixel_format(decodePixelFormat(getPixelFormat()));

        LayerProtoHelper::writeToProto(getColor(), [&]() { return layerInfo->mutable_color(); });
@@ -1884,7 +1918,6 @@ void Layer::writeToProto(LayerProto* layerInfo, LayerVector::StateSet stateSet, 
                                       [&]() { return layerInfo->mutable_requested_color(); });

        layerInfo->set_flags(state.flags);



        LayerProtoHelper::writeToProto(transform, layerInfo->mutable_transform());

        LayerProtoHelper::writeToProto(requestedTransform,

                                       layerInfo->mutable_requested_transform());
@@ -1901,29 +1934,6 @@ void Layer::writeToProto(LayerProto* layerInfo, LayerVector::StateSet stateSet, 
        } else {

            layerInfo->set_z_order_relative_of(-1);

        }



        auto buffer = mActiveBuffer;

        if (buffer != nullptr) {

            LayerProtoHelper::writeToProto(buffer,

                                           [&]() { return layerInfo->mutable_active_buffer(); });

            LayerProtoHelper::writeToProto(ui::Transform(mCurrentTransform),

                                           layerInfo->mutable_buffer_transform());

        }



        layerInfo->set_queued_frames(getQueuedFrameCount());

        layerInfo->set_refresh_pending(isBufferLatched());

        layerInfo->set_curr_frame(mCurrentFrameNumber);

        layerInfo->set_effective_scaling_mode(getEffectiveScalingMode());



        for (const auto& pendingState : mPendingStates) {

            auto barrierLayer = pendingState.barrierLayer_legacy.promote();

            if (barrierLayer != nullptr) {

                BarrierLayerProto* barrierLayerProto = layerInfo->add_barrier_layer();

                barrierLayerProto->set_id(barrierLayer->sequence);

                barrierLayerProto->set_frame_number(pendingState.frameNumber_legacy);

            }

        }

        LayerProtoHelper::writeToProto(mBounds, [&]() { return layerInfo->mutable_bounds(); });

    }



    if (traceFlags & SurfaceTracing::TRACE_INPUT) {
@@ -1936,23 +1946,19 @@ void Layer::writeToProto(LayerProto* layerInfo, LayerVector::StateSet stateSet, 
        for (const auto& entry : state.metadata.mMap) {

            (*protoMap)[entry.first] = std::string(entry.second.cbegin(), entry.second.cend());

        }

        LayerProtoHelper::writeToProto(mEffectiveTransform,

                                       layerInfo->mutable_effective_transform());

        LayerProtoHelper::writeToProto(mSourceBounds,

                                       [&]() { return layerInfo->mutable_source_bounds(); });

        LayerProtoHelper::writeToProto(mScreenBounds,

                                       [&]() { return layerInfo->mutable_screen_bounds(); });

    }

}



void Layer::writeToProto(LayerProto* layerInfo, const sp<DisplayDevice>& displayDevice,

                         uint32_t traceFlags) {

void Layer::writeToProtoCompositionState(LayerProto* layerInfo,

                                         const sp<DisplayDevice>& displayDevice,

                                         uint32_t traceFlags) const {

    auto outputLayer = findOutputLayerForDisplay(displayDevice);

    if (!outputLayer) {

        return;

    }



    writeToProto(layerInfo, LayerVector::StateSet::Drawing, traceFlags);

    writeToProtoDrawingState(layerInfo, traceFlags);

    writeToProtoCommonState(layerInfo, LayerVector::StateSet::Drawing, traceFlags);



    const auto& compositionState = outputLayer->getState();
@@ -1970,13 +1976,6 @@ void Layer::writeToProto(LayerProto* layerInfo, const sp<DisplayDevice>& display 
            static_cast<int32_t>(compositionState.hwc ? (*compositionState.hwc).hwcCompositionType

                                                      : Hwc2::IComposerClient::Composition::CLIENT);

    layerInfo->set_hwc_composition_type(compositionType);



    if (std::strcmp(getTypeId(), "BufferLayer") == 0 &&

        static_cast<BufferLayer*>(this)->isProtected()) {

        layerInfo->set_is_protected(true);

    } else {

        layerInfo->set_is_protected(false);

    }

}



bool Layer::isRemovedFromCurrentState() const  {

services/surfaceflinger/Layer.h

+22 −10
Original line number Diff line number Diff line
@@ -435,11 +435,21 @@ public: 


    bool isRemovedFromCurrentState() const;



    void writeToProto(LayerProto* layerInfo, LayerVector::StateSet stateSet,

                      uint32_t traceFlags = SurfaceTracing::TRACE_ALL);



    void writeToProto(LayerProto* layerInfo, const sp<DisplayDevice>& displayDevice,

                      uint32_t traceFlags = SurfaceTracing::TRACE_ALL);

    // Write states that are modified by the main thread. This includes drawing

    // state as well as buffer data. This should be called in the main or tracing

    // thread.

    void writeToProtoDrawingState(LayerProto* layerInfo,

                                  uint32_t traceFlags = SurfaceTracing::TRACE_ALL) const;

    // Write states that are modified by the main thread. This includes drawing

    // state as well as buffer data and composition data for layers on the specified

    // display. This should be called in the main or tracing thread.

    void writeToProtoCompositionState(LayerProto* layerInfo, const sp<DisplayDevice>& displayDevice,

                                      uint32_t traceFlags = SurfaceTracing::TRACE_ALL) const;

    // Write drawing or current state. If writing current state, the caller should hold the

    // external mStateLock. If writing drawing state, this function should be called on the

    // main or tracing thread.

    void writeToProtoCommonState(LayerProto* layerInfo, LayerVector::StateSet stateSet,

                                 uint32_t traceFlags = SurfaceTracing::TRACE_ALL) const;



    virtual Geometry getActiveGeometry(const Layer::State& s) const { return s.active_legacy; }

    virtual uint32_t getActiveWidth(const Layer::State& s) const { return s.active_legacy.w; }
@@ -819,13 +829,15 @@ protected: 


    bool mPrimaryDisplayOnly = false;



    // these are protected by an external lock

    State mCurrentState;

    // These are only accessed by the main thread or the tracing thread.

    State mDrawingState;

    std::atomic<uint32_t> mTransactionFlags{0};

    // Store a copy of the pending state so that the drawing thread can access the

    // states without a lock.

    Vector<State> mPendingStatesSnapshot;



    // Accessed from main thread and binder threads

    Mutex mPendingStateMutex;

    // these are protected by an external lock (mStateLock)

    State mCurrentState;

    std::atomic<uint32_t> mTransactionFlags{0};

    Vector<State> mPendingStates;



    // Timestamp history for UIAutomation. Thread safe.

services/surfaceflinger/SurfaceFlinger.cpp

+22 −21
Original line number Diff line number Diff line
@@ -4578,18 +4578,22 @@ status_t SurfaceFlinger::doDump(int fd, const DumpArgs& args, 


        if (const auto it = dumpers.find(flag); it != dumpers.end()) {

            (it->second)(args, asProto, result);

        } else {

            if (asProto) {

                LayersProto layersProto = dumpProtoInfo(LayerVector::StateSet::Current);

                result.append(layersProto.SerializeAsString().c_str(), layersProto.ByteSize());

            } else {

        } else if (!asProto) {

            dumpAllLocked(args, result);

        }

        }



        if (locked) {

            mStateLock.unlock();

        }



        LayersProto layersProto = dumpProtoFromMainThread();

        if (asProto) {

            result.append(layersProto.SerializeAsString().c_str(), layersProto.ByteSize());

        } else {

            auto layerTree = LayerProtoParser::generateLayerTree(layersProto);

            result.append(LayerProtoParser::layerTreeToString(layerTree));

            result.append("\n");

        }

    }

    write(fd, result.c_str(), result.size());

    return NO_ERROR;
@@ -4830,19 +4834,23 @@ void SurfaceFlinger::dumpWideColorInfo(std::string& result) const { 
    result.append("\n");

}



LayersProto SurfaceFlinger::dumpProtoInfo(LayerVector::StateSet stateSet,

                                          uint32_t traceFlags) const {

LayersProto SurfaceFlinger::dumpDrawingStateProto(uint32_t traceFlags) const {

    LayersProto layersProto;

    const bool useDrawing = stateSet == LayerVector::StateSet::Drawing;

    const State& state = useDrawing ? mDrawingState : mCurrentState;

    state.traverseInZOrder([&](Layer* layer) {

    mDrawingState.traverseInZOrder([&](Layer* layer) {

        LayerProto* layerProto = layersProto.add_layers();

        layer->writeToProto(layerProto, stateSet, traceFlags);

        layer->writeToProtoDrawingState(layerProto, traceFlags);

        layer->writeToProtoCommonState(layerProto, LayerVector::StateSet::Drawing, traceFlags);

    });



    return layersProto;

}



LayersProto SurfaceFlinger::dumpProtoFromMainThread(uint32_t traceFlags) {

    LayersProto layersProto;

    postMessageSync(new LambdaMessage([&]() { layersProto = dumpDrawingStateProto(traceFlags); }));

    return layersProto;

}



LayersProto SurfaceFlinger::dumpVisibleLayersProtoInfo(

        const sp<DisplayDevice>& displayDevice) const {

    LayersProto layersProto;
@@ -4863,7 +4871,7 @@ LayersProto SurfaceFlinger::dumpVisibleLayersProtoInfo( 
    mDrawingState.traverseInZOrder([&](Layer* layer) {

        if (!layer->visibleRegion.isEmpty() && !display->getOutputLayersOrderedByZ().empty()) {

            LayerProto* layerProto = layersProto.add_layers();

            layer->writeToProto(layerProto, displayDevice);

            layer->writeToProtoCompositionState(layerProto, displayDevice);

        }

    });
@@ -4927,13 +4935,6 @@ void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, std::string& result) co 
                  mGraphicBufferProducerList.size(), mMaxGraphicBufferProducerListSize);

    colorizer.reset(result);



    {

        LayersProto layersProto = dumpProtoInfo(LayerVector::StateSet::Current);

        auto layerTree = LayerProtoParser::generateLayerTree(layersProto);

        result.append(LayerProtoParser::layerTreeToString(layerTree));

        result.append("\n");

    }



    {

        StringAppendF(&result, "Composition layers\n");

        mDrawingState.traverseInZOrder([&](Layer* layer) {

services/surfaceflinger/SurfaceFlinger.h

+5 −3
Original line number Diff line number Diff line
@@ -265,7 +265,8 @@ public: 
    status_t postMessageAsync(const sp<MessageBase>& msg, nsecs_t reltime = 0, uint32_t flags = 0);



    // post a synchronous message to the main thread

    status_t postMessageSync(const sp<MessageBase>& msg, nsecs_t reltime = 0, uint32_t flags = 0);

    status_t postMessageSync(const sp<MessageBase>& msg, nsecs_t reltime = 0, uint32_t flags = 0)

            EXCLUDES(mStateLock);



    // force full composition on all displays

    void repaintEverything();
@@ -896,8 +897,9 @@ private: 
    void dumpBufferingStats(std::string& result) const;

    void dumpDisplayIdentificationData(std::string& result) const;

    void dumpWideColorInfo(std::string& result) const;

    LayersProto dumpProtoInfo(LayerVector::StateSet stateSet,

                              uint32_t traceFlags = SurfaceTracing::TRACE_ALL) const;

    LayersProto dumpDrawingStateProto(uint32_t traceFlags = SurfaceTracing::TRACE_ALL) const;

    LayersProto dumpProtoFromMainThread(uint32_t traceFlags = SurfaceTracing::TRACE_ALL)

            EXCLUDES(mStateLock);

    void withTracingLock(std::function<void()> operation) REQUIRES(mStateLock);

    LayersProto dumpVisibleLayersProtoInfo(const sp<DisplayDevice>& display) const;

services/surfaceflinger/SurfaceTracing.cpp

+1 −1
Original line number Diff line number Diff line
@@ -162,7 +162,7 @@ LayersTraceProto SurfaceTracing::traceLayersLocked(const char* where) { 
    LayersTraceProto entry;

    entry.set_elapsed_realtime_nanos(elapsedRealtimeNano());

    entry.set_where(where);

    LayersProto layers(mFlinger.dumpProtoInfo(LayerVector::StateSet::Drawing, mTraceFlags));

    LayersProto layers(mFlinger.dumpDrawingStateProto(mTraceFlags));

    entry.mutable_layers()->Swap(&layers);



    return entry;