Merge "Fix deadlock of main thread and Perfetto thread (LayerDataSource::OnStart)" into main
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 65a0ed3..06560d0 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -946,16 +946,20 @@
}));
}));
- mLayerTracing.setTakeLayersSnapshotProtoFunction([&](uint32_t traceFlags) {
- auto snapshot = perfetto::protos::LayersSnapshotProto{};
- mScheduler
- ->schedule([&]() FTL_FAKE_GUARD(mStateLock) FTL_FAKE_GUARD(kMainThreadContext) {
- snapshot = takeLayersSnapshotProto(traceFlags, TimePoint::now(),
- mLastCommittedVsyncId, true);
- })
- .wait();
- return snapshot;
- });
+ mLayerTracing.setTakeLayersSnapshotProtoFunction(
+ [&](uint32_t traceFlags,
+ const LayerTracing::OnLayersSnapshotCallback& onLayersSnapshot) {
+ // Do not wait the future to avoid deadlocks
+ // between main and Perfetto threads (b/313130597)
+ static_cast<void>(mScheduler->schedule(
+ [&, onLayersSnapshot]() FTL_FAKE_GUARD(mStateLock)
+ FTL_FAKE_GUARD(kMainThreadContext) {
+ auto snapshot =
+ takeLayersSnapshotProto(traceFlags, TimePoint::now(),
+ mLastCommittedVsyncId, true);
+ onLayersSnapshot(std::move(snapshot));
+ }));
+ });
// Commit secondary display(s).
processDisplayChangesLocked();
diff --git a/services/surfaceflinger/Tracing/LayerDataSource.cpp b/services/surfaceflinger/Tracing/LayerDataSource.cpp
index ed1e2ec..cc0063c 100644
--- a/services/surfaceflinger/Tracing/LayerDataSource.cpp
+++ b/services/surfaceflinger/Tracing/LayerDataSource.cpp
@@ -82,10 +82,13 @@
}
}
-void LayerDataSource::OnStop(const LayerDataSource::StopArgs&) {
+void LayerDataSource::OnStop(const LayerDataSource::StopArgs& args) {
ALOGD("Received OnStop event (mode = 0x%02x, flags = 0x%02x)", mMode, mFlags);
if (auto* p = mLayerTracing.load()) {
- p->onStop(mMode);
+ // In dump mode we need to defer the stop (through HandleStopAsynchronously()) till
+ // the layers snapshot has been captured and written to perfetto. We must avoid writing
+ // to perfetto within the OnStop callback to prevent deadlocks (b/313130597).
+ p->onStop(mMode, mFlags, args.HandleStopAsynchronously());
}
}
diff --git a/services/surfaceflinger/Tracing/LayerTracing.cpp b/services/surfaceflinger/Tracing/LayerTracing.cpp
index d40b888..d78f9bb 100644
--- a/services/surfaceflinger/Tracing/LayerTracing.cpp
+++ b/services/surfaceflinger/Tracing/LayerTracing.cpp
@@ -32,7 +32,7 @@
namespace android {
LayerTracing::LayerTracing() {
- mTakeLayersSnapshotProto = [](uint32_t) { return perfetto::protos::LayersSnapshotProto{}; };
+ mTakeLayersSnapshotProto = [](uint32_t, const OnLayersSnapshotCallback&) {};
LayerDataSource::Initialize(*this);
}
@@ -45,7 +45,7 @@
}
void LayerTracing::setTakeLayersSnapshotProtoFunction(
- const std::function<perfetto::protos::LayersSnapshotProto(uint32_t)>& callback) {
+ const std::function<void(uint32_t, const OnLayersSnapshotCallback&)>& callback) {
mTakeLayersSnapshotProto = callback;
}
@@ -62,7 +62,10 @@
// It might take a while before a layers change occurs and a "spontaneous" snapshot is
// taken. Let's manually take a snapshot, so that the trace's first entry will contain
// the current layers state.
- addProtoSnapshotToOstream(mTakeLayersSnapshotProto(flags), Mode::MODE_ACTIVE);
+ auto onLayersSnapshot = [this](perfetto::protos::LayersSnapshotProto&& snapshot) {
+ addProtoSnapshotToOstream(std::move(snapshot), Mode::MODE_ACTIVE);
+ };
+ mTakeLayersSnapshotProto(flags, onLayersSnapshot);
ALOGD("Started active tracing (traced initial snapshot)");
break;
}
@@ -89,9 +92,7 @@
break;
}
case Mode::MODE_DUMP: {
- auto snapshot = mTakeLayersSnapshotProto(flags);
- addProtoSnapshotToOstream(std::move(snapshot), Mode::MODE_DUMP);
- ALOGD("Started dump tracing (dumped single snapshot)");
+ ALOGD("Started dump tracing");
break;
}
default: {
@@ -125,10 +126,27 @@
ALOGD("Flushed generated tracing");
}
-void LayerTracing::onStop(Mode mode) {
- if (mode == Mode::MODE_ACTIVE) {
- mIsActiveTracingStarted.store(false);
- ALOGD("Stopped active tracing");
+void LayerTracing::onStop(Mode mode, uint32_t flags, std::function<void()>&& deferredStopDone) {
+ switch (mode) {
+ case Mode::MODE_ACTIVE: {
+ mIsActiveTracingStarted.store(false);
+ deferredStopDone();
+ ALOGD("Stopped active tracing");
+ break;
+ }
+ case Mode::MODE_DUMP: {
+ auto onLayersSnapshot = [this, deferredStopDone = std::move(deferredStopDone)](
+ perfetto::protos::LayersSnapshotProto&& snapshot) {
+ addProtoSnapshotToOstream(std::move(snapshot), Mode::MODE_DUMP);
+ deferredStopDone();
+ ALOGD("Stopped dump tracing (written single snapshot)");
+ };
+ mTakeLayersSnapshotProto(flags, onLayersSnapshot);
+ break;
+ }
+ default: {
+ deferredStopDone();
+ }
}
}
diff --git a/services/surfaceflinger/Tracing/LayerTracing.h b/services/surfaceflinger/Tracing/LayerTracing.h
index 2895ba7..e99fe4c 100644
--- a/services/surfaceflinger/Tracing/LayerTracing.h
+++ b/services/surfaceflinger/Tracing/LayerTracing.h
@@ -87,6 +87,7 @@
class LayerTracing {
public:
using Mode = perfetto::protos::pbzero::SurfaceFlingerLayersConfig::Mode;
+ using OnLayersSnapshotCallback = std::function<void(perfetto::protos::LayersSnapshotProto&&)>;
enum Flag : uint32_t {
TRACE_INPUT = 1 << 1,
@@ -102,7 +103,7 @@
LayerTracing(std::ostream&);
~LayerTracing();
void setTakeLayersSnapshotProtoFunction(
- const std::function<perfetto::protos::LayersSnapshotProto(uint32_t)>&);
+ const std::function<void(uint32_t, const OnLayersSnapshotCallback&)>&);
void setTransactionTracing(TransactionTracing&);
// Start event from perfetto data source
@@ -110,7 +111,7 @@
// Flush event from perfetto data source
void onFlush(Mode mode, uint32_t flags, bool isBugreport);
// Stop event from perfetto data source
- void onStop(Mode mode);
+ void onStop(Mode mode, uint32_t flags, std::function<void()>&& deferredStopDone);
void addProtoSnapshotToOstream(perfetto::protos::LayersSnapshotProto&& snapshot, Mode mode);
bool isActiveTracingStarted() const;
@@ -123,7 +124,7 @@
void writeSnapshotToPerfetto(const perfetto::protos::LayersSnapshotProto& snapshot, Mode mode);
bool checkAndUpdateLastVsyncIdWrittenToPerfetto(Mode mode, std::int64_t vsyncId);
- std::function<perfetto::protos::LayersSnapshotProto(uint32_t)> mTakeLayersSnapshotProto;
+ std::function<void(uint32_t, const OnLayersSnapshotCallback&)> mTakeLayersSnapshotProto;
TransactionTracing* mTransactionTracing;
std::atomic<bool> mIsActiveTracingStarted{false};