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: Ib2b49aedf06ee5a262d3162366ddf75d08432e05
Merged-In: I8bb93e9b9f81faec59585b770eb7ba0fbcd9b51b
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 1318bc0..f4284fe 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -153,7 +153,6 @@
mRemoteSyncPoints.clear();
{
- Mutex::Autolock pendingStateLock(mPendingStateMutex);
for (State pendingState : mPendingStates) {
pendingState.barrierLayer_legacy = nullptr;
}
@@ -907,6 +906,7 @@
// Commit the transaction
commitTransaction(c);
+ mPendingStatesSnapshot = mPendingStates;
mCurrentState.callbackHandles = {};
return flags;
}
@@ -1874,14 +1874,61 @@
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);
@@ -1901,17 +1948,10 @@
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();
@@ -1922,15 +1962,9 @@
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(); });
@@ -1938,7 +1972,6 @@
[&]() { return layerInfo->mutable_requested_color(); });
layerInfo->set_flags(state.flags);
- LayerProtoHelper::writeToProto(transform, layerInfo->mutable_transform());
LayerProtoHelper::writeToProto(requestedTransform,
layerInfo->mutable_requested_transform());
@@ -1955,29 +1988,6 @@
} 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) {
@@ -1990,23 +2000,19 @@
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();
@@ -2024,13 +2030,6 @@
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 {