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

Commit d7ff85f5 authored by Vishnu Nair's avatar Vishnu Nair
Browse files

Fix deadlock when calling dumpsys surfaceflinger 

We were holding the state lock when calling into the main thread to
access some debug data. The main thread was stuck waiting for the
state lock, causing the deadlock.

Fix by cleaning up the surface flinger dumpsys and reduce the scope
of the state lock.

Fixes: 313910768, 300525768
Test: dumpsys SurfaceFlinger
Test: dumpsys SurfaceFlinger --hwclayers
Test: dumpsys SurfaceFlinger --frontend
Test: dumpsys SurfaceFlinger --timestats --proto
Change-Id: Id62b9427a14bc37bc92759fbaa8ca2b3da94905d
parent 8597755c

services/surfaceflinger/SurfaceFlinger.cpp

+134 −161
Original line number Diff line number Diff line
@@ -5951,20 +5951,22 @@ status_t SurfaceFlinger::doDump(int fd, const DumpArgs& args, bool asProto) { 
            !PermissionCache::checkPermission(sDump, pid, uid)) {

        StringAppendF(&result, "Permission Denial: can't dump SurfaceFlinger from pid=%d, uid=%d\n",

                      pid, uid);

    } else {

        Dumper hwclayersDump = [this](const DumpArgs&, bool, std::string& result)

                                       FTL_FAKE_GUARD(mStateLock) -> void const {

            if (mLayerLifecycleManagerEnabled) {

                mScheduler

                        ->schedule([this, &result]() FTL_FAKE_GUARD(kMainThreadContext)

                                           FTL_FAKE_GUARD(mStateLock) {

                                               dumpHwcLayersMinidump(result);

                                           })

                        .get();

            } else {

                dumpHwcLayersMinidumpLockedLegacy(result);

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

        return NO_ERROR;

    }



    if (asProto && args.empty()) {

        perfetto::protos::LayersTraceFileProto traceFileProto =

                mLayerTracing.createTraceFileProto();

        perfetto::protos::LayersSnapshotProto* layersTrace = traceFileProto.add_entry();

        perfetto::protos::LayersProto layersProto = dumpProtoFromMainThread();

        layersTrace->mutable_layers()->Swap(&layersProto);

        auto displayProtos = dumpDisplayProto();

        layersTrace->mutable_displays()->Swap(&displayProtos);

        result.append(traceFileProto.SerializeAsString());

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

        return NO_ERROR;

    }

        };



    static const std::unordered_map<std::string, Dumper> dumpers = {

            {"--comp-displays"s, dumper(&SurfaceFlinger::dumpCompositionDisplays)},
@@ -5973,8 +5975,9 @@ status_t SurfaceFlinger::doDump(int fd, const DumpArgs& args, bool asProto) { 
            {"--edid"s, argsDumper(&SurfaceFlinger::dumpRawDisplayIdentificationData)},

            {"--events"s, dumper(&SurfaceFlinger::dumpEvents)},

            {"--frametimeline"s, argsDumper(&SurfaceFlinger::dumpFrameTimeline)},

            {"--frontend"s, mainThreadDumper(&SurfaceFlinger::dumpFrontEnd)},

            {"--hdrinfo"s, dumper(&SurfaceFlinger::dumpHdrInfo)},

                {"--hwclayers"s, std::move(hwclayersDump)},

            {"--hwclayers"s, mainThreadDumper(&SurfaceFlinger::dumpHwcLayersMinidump)},

            {"--latency"s, argsDumper(&SurfaceFlinger::dumpStatsLocked)},

            {"--latency-clear"s, argsDumper(&SurfaceFlinger::clearStatsLocked)},

            {"--list"s, dumper(&SurfaceFlinger::listLayersLocked)},
@@ -5983,10 +5986,14 @@ status_t SurfaceFlinger::doDump(int fd, const DumpArgs& args, bool asProto) { 
            {"--timestats"s, protoDumper(&SurfaceFlinger::dumpTimeStats)},

            {"--vsync"s, dumper(&SurfaceFlinger::dumpVsync)},

            {"--wide-color"s, dumper(&SurfaceFlinger::dumpWideColorInfo)},

                {"--frontend"s, dumper(&SurfaceFlinger::dumpFrontEnd)},

    };



    const auto flag = args.empty() ? ""s : std::string(String8(args[0]));

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

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

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

        return NO_ERROR;

    }



    // Traversal of drawing state must happen on the main thread.

    // Otherwise, SortedVector may have shared ownership during concurrent
@@ -5994,97 +6001,10 @@ status_t SurfaceFlinger::doDump(int fd, const DumpArgs& args, bool asProto) { 
    std::string compositionLayers;

    mScheduler

            ->schedule([&]() FTL_FAKE_GUARD(mStateLock) FTL_FAKE_GUARD(kMainThreadContext) {

                    if (!mLayerLifecycleManagerEnabled) {

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

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

                            auto* compositionState = layer->getCompositionState();

                            if (!compositionState || !compositionState->isVisible) return;

                            android::base::StringAppendF(&compositionLayers, "* Layer %p (%s)\n",

                                                         layer,

                                                         layer->getDebugName()

                                                                 ? layer->getDebugName()

                                                                 : "<unknown>");

                            compositionState->dump(compositionLayers);

                        });

                    } else {

                        std::ostringstream out;

                        out << "\nComposition list\n";

                        ui::LayerStack lastPrintedLayerStackHeader = ui::INVALID_LAYER_STACK;

                        mLayerSnapshotBuilder.forEachVisibleSnapshot(

                                [&](std::unique_ptr<frontend::LayerSnapshot>& snapshot) {

                                    if (snapshot->hasSomethingToDraw()) {

                                        if (lastPrintedLayerStackHeader !=

                                            snapshot->outputFilter.layerStack) {

                                            lastPrintedLayerStackHeader =

                                                    snapshot->outputFilter.layerStack;

                                            out << "LayerStack=" << lastPrintedLayerStackHeader.id

                                                << "\n";

                                        }

                                        out << "  " << *snapshot << "\n";

                                    }

                                });



                        out << "\nInput list\n";

                        lastPrintedLayerStackHeader = ui::INVALID_LAYER_STACK;

                        mLayerSnapshotBuilder.forEachInputSnapshot(

                                [&](const frontend::LayerSnapshot& snapshot) {

                                    if (lastPrintedLayerStackHeader !=

                                        snapshot.outputFilter.layerStack) {

                                        lastPrintedLayerStackHeader =

                                                snapshot.outputFilter.layerStack;

                                        out << "LayerStack=" << lastPrintedLayerStackHeader.id

                                            << "\n";

                                    }

                                    out << "  " << snapshot << "\n";

                                });



                        out << "\nLayer Hierarchy\n"

                            << mLayerHierarchyBuilder.getHierarchy()

                            << "\nOffscreen Hierarchy\n"

                            << mLayerHierarchyBuilder.getOffscreenHierarchy() << "\n\n";

                        compositionLayers = out.str();

                        dumpHwcLayersMinidump(compositionLayers);

                    }

                dumpVisibleFrontEnd(compositionLayers);

            })

            .get();



        bool dumpLayers = true;

        {

            TimedLock lock(mStateLock, s2ns(1), __func__);

            if (!lock.locked()) {

                StringAppendF(&result, "Dumping without lock after timeout: %s (%d)\n",

                              strerror(-lock.status), lock.status);

            }



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

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

                dumpLayers = false;

            } else if (!asProto) {

                dumpAllLocked(args, compositionLayers, result);

            }

        }



        if (dumpLayers) {

            perfetto::protos::LayersTraceFileProto traceFileProto =

                    mLayerTracing.createTraceFileProto();

            perfetto::protos::LayersSnapshotProto* layersTrace = traceFileProto.add_entry();

            perfetto::protos::LayersProto layersProto = dumpProtoFromMainThread();

            layersTrace->mutable_layers()->Swap(&layersProto);

            auto displayProtos = dumpDisplayProto();

            layersTrace->mutable_displays()->Swap(&displayProtos);



            if (asProto) {

                result.append(traceFileProto.SerializeAsString());

            } else {

                // Dump info that we need to access from the main thread

                const auto layerTree = LayerProtoParser::generateLayerTree(layersTrace->layers());

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

                result.append("\n");

                dumpOffscreenLayers(result);

            }

        }

    }



    dumpAll(args, compositionLayers, result);

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

    return NO_ERROR;

}
@@ -6296,8 +6216,6 @@ void SurfaceFlinger::dumpHdrInfo(std::string& result) const { 
}



void SurfaceFlinger::dumpFrontEnd(std::string& result) {

    mScheduler

            ->schedule([&]() FTL_FAKE_GUARD(mStateLock) FTL_FAKE_GUARD(kMainThreadContext) {

    std::ostringstream out;

    out << "\nComposition list\n";

    ui::LayerStack lastPrintedLayerStackHeader = ui::INVALID_LAYER_STACK;
@@ -6311,8 +6229,7 @@ void SurfaceFlinger::dumpFrontEnd(std::string& result) { 


    out << "\nInput list\n";

    lastPrintedLayerStackHeader = ui::INVALID_LAYER_STACK;

                mLayerSnapshotBuilder.forEachInputSnapshot(

                        [&](const frontend::LayerSnapshot& snapshot) {

    mLayerSnapshotBuilder.forEachInputSnapshot([&](const frontend::LayerSnapshot& snapshot) {

        if (lastPrintedLayerStackHeader != snapshot.outputFilter.layerStack) {

            lastPrintedLayerStackHeader = snapshot.outputFilter.layerStack;

            out << "LayerStack=" << lastPrintedLayerStackHeader.id << "\n";
@@ -6321,12 +6238,59 @@ void SurfaceFlinger::dumpFrontEnd(std::string& result) { 
    });



    out << "\nLayer Hierarchy\n"

                    << mLayerHierarchyBuilder.getHierarchy().dump()

                    << "\nOffscreen Hierarchy\n"

        << mLayerHierarchyBuilder.getHierarchy().dump() << "\nOffscreen Hierarchy\n"

        << mLayerHierarchyBuilder.getOffscreenHierarchy().dump() << "\n\n";

    result.append(out.str());

            })

            .get();

}



void SurfaceFlinger::dumpVisibleFrontEnd(std::string& result) {

    if (!mLayerLifecycleManagerEnabled) {

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

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

            auto* compositionState = layer->getCompositionState();

            if (!compositionState || !compositionState->isVisible) return;

            android::base::StringAppendF(&result, "* Layer %p (%s)\n", layer,

                                         layer->getDebugName() ? layer->getDebugName()

                                                               : "<unknown>");

            compositionState->dump(result);

        });



        StringAppendF(&result, "Offscreen Layers\n");

        for (Layer* offscreenLayer : mOffscreenLayers) {

            offscreenLayer->traverse(LayerVector::StateSet::Drawing,

                                     [&](Layer* layer) { layer->dumpOffscreenDebugInfo(result); });

        }

    } else {

        std::ostringstream out;

        out << "\nComposition list\n";

        ui::LayerStack lastPrintedLayerStackHeader = ui::INVALID_LAYER_STACK;

        mLayerSnapshotBuilder.forEachVisibleSnapshot(

                [&](std::unique_ptr<frontend::LayerSnapshot>& snapshot) {

                    if (snapshot->hasSomethingToDraw()) {

                        if (lastPrintedLayerStackHeader != snapshot->outputFilter.layerStack) {

                            lastPrintedLayerStackHeader = snapshot->outputFilter.layerStack;

                            out << "LayerStack=" << lastPrintedLayerStackHeader.id << "\n";

                        }

                        out << "  " << *snapshot << "\n";

                    }

                });



        out << "\nInput list\n";

        lastPrintedLayerStackHeader = ui::INVALID_LAYER_STACK;

        mLayerSnapshotBuilder.forEachInputSnapshot([&](const frontend::LayerSnapshot& snapshot) {

            if (lastPrintedLayerStackHeader != snapshot.outputFilter.layerStack) {

                lastPrintedLayerStackHeader = snapshot.outputFilter.layerStack;

                out << "LayerStack=" << lastPrintedLayerStackHeader.id << "\n";

            }

            out << "  " << snapshot << "\n";

        });



        out << "\nLayer Hierarchy\n"

            << mLayerHierarchyBuilder.getHierarchy() << "\nOffscreen Hierarchy\n"

            << mLayerHierarchyBuilder.getOffscreenHierarchy() << "\n\n";

        result = out.str();

        dumpHwcLayersMinidump(result);

    }

}



perfetto::protos::LayersProto SurfaceFlinger::dumpDrawingStateProto(uint32_t traceFlags) const {
@@ -6427,7 +6391,7 @@ void SurfaceFlinger::dumpOffscreenLayers(std::string& result) { 
}



void SurfaceFlinger::dumpHwcLayersMinidumpLockedLegacy(std::string& result) const {

    for (const auto& [token, display] : mDisplays) {

    for (const auto& [token, display] : FTL_FAKE_GUARD(mStateLock, mDisplays)) {

        const auto displayId = HalDisplayId::tryCast(display->getId());

        if (!displayId) {

            continue;
@@ -6444,7 +6408,10 @@ void SurfaceFlinger::dumpHwcLayersMinidumpLockedLegacy(std::string& result) cons 
}



void SurfaceFlinger::dumpHwcLayersMinidump(std::string& result) const {

    for (const auto& [token, display] : mDisplays) {

    if (!mLayerLifecycleManagerEnabled) {

        return dumpHwcLayersMinidumpLockedLegacy(result);

    }

    for (const auto& [token, display] : FTL_FAKE_GUARD(mStateLock, mDisplays)) {

        const auto displayId = HalDisplayId::tryCast(display->getId());

        if (!displayId) {

            continue;
@@ -6469,8 +6436,14 @@ void SurfaceFlinger::dumpHwcLayersMinidump(std::string& result) const { 
    }

}



void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, const std::string& compositionLayers,

void SurfaceFlinger::dumpAll(const DumpArgs& args, const std::string& compositionLayers,

                             std::string& result) const {

    TimedLock lock(mStateLock, s2ns(1), __func__);

    if (!lock.locked()) {

        StringAppendF(&result, "Dumping without lock after timeout: %s (%d)\n",

                      strerror(-lock.status), lock.status);

    }



    const bool colorize = !args.empty() && args[0] == String16("--color");

    Colorizer colorizer(colorize);

services/surfaceflinger/SurfaceFlinger.h

+34 −6
Original line number Diff line number Diff line
@@ -21,6 +21,8 @@ 
 * NOTE: Make sure this file doesn't include  anything from <gl/ > or <gl2/ >

 */



#include <android-base/stringprintf.h>

#include <android-base/strings.h>

#include <android-base/thread_annotations.h>

#include <android/gui/BnSurfaceComposer.h>

#include <android/gui/DisplayStatInfo.h>
@@ -77,6 +79,7 @@ 
#include "FrontEnd/LayerSnapshotBuilder.h"

#include "FrontEnd/TransactionHandler.h"

#include "LayerVector.h"

#include "MutexUtils.h"

#include "Scheduler/ISchedulerCallback.h"

#include "Scheduler/RefreshRateSelector.h"

#include "Scheduler/RefreshRateStats.h"
@@ -478,22 +481,46 @@ private: 
        return std::bind(std::forward<F>(dump), _3);

    }



    Dumper lockedDumper(Dumper dump) {

        return [this, dump](const DumpArgs& args, bool asProto, std::string& result) -> void {

            TimedLock lock(mStateLock, s2ns(1), __func__);

            if (!lock.locked()) {

                base::StringAppendF(&result, "Dumping without lock after timeout: %s (%d)\n",

                                    strerror(-lock.status), lock.status);

            }

            dump(args, asProto, result);

        };

    }



    template <typename F, std::enable_if_t<std::is_member_function_pointer_v<F>>* = nullptr>

    Dumper dumper(F dump) {

        using namespace std::placeholders;

        return std::bind(dump, this, _3);

        return lockedDumper(std::bind(dump, this, _3));

    }



    template <typename F>

    Dumper argsDumper(F dump) {

        using namespace std::placeholders;

        return std::bind(dump, this, _1, _3);

        return lockedDumper(std::bind(dump, this, _1, _3));

    }



    template <typename F>

    Dumper protoDumper(F dump) {

        using namespace std::placeholders;

        return std::bind(dump, this, _1, _2, _3);

        return lockedDumper(std::bind(dump, this, _1, _2, _3));

    }



    template <typename F, std::enable_if_t<std::is_member_function_pointer_v<F>>* = nullptr>

    Dumper mainThreadDumper(F dump) {

        using namespace std::placeholders;

        Dumper dumper = std::bind(dump, this, _3);

        return [this, dumper](const DumpArgs& args, bool asProto, std::string& result) -> void {

            mScheduler

                    ->schedule(

                            [&args, asProto, &result, dumper]() FTL_FAKE_GUARD(kMainThreadContext)

                                    FTL_FAKE_GUARD(mStateLock) { dumper(args, asProto, result); })

                    .get();

        };

    }



    // Maximum allowed number of display frames that can be set through backdoor
@@ -1080,8 +1107,8 @@ private: 
    /*

     * Debugging & dumpsys

     */

    void dumpAllLocked(const DumpArgs& args, const std::string& compositionLayers,

                       std::string& result) const REQUIRES(mStateLock);

    void dumpAll(const DumpArgs& args, const std::string& compositionLayers,

                 std::string& result) const EXCLUDES(mStateLock);

    void dumpHwcLayersMinidump(std::string& result) const REQUIRES(mStateLock, kMainThreadContext);

    void dumpHwcLayersMinidumpLockedLegacy(std::string& result) const REQUIRES(mStateLock);
@@ -1103,7 +1130,8 @@ private: 
    void dumpRawDisplayIdentificationData(const DumpArgs&, std::string& result) const;

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

    void dumpHdrInfo(std::string& result) const REQUIRES(mStateLock);

    void dumpFrontEnd(std::string& result);

    void dumpFrontEnd(std::string& result) REQUIRES(kMainThreadContext);

    void dumpVisibleFrontEnd(std::string& result) REQUIRES(mStateLock, kMainThreadContext);



    perfetto::protos::LayersProto dumpDrawingStateProto(uint32_t traceFlags) const;

    void dumpOffscreenLayersProto(perfetto::protos::LayersProto& layersProto,