Skip to content

Commit cbec8f7

Browse files
Melody Hsuandroid-build-merge-worker-robot
authored andcommitted
Merge "Fix SurfaceFlinger crash caused by layerleak" into main am: c8ca54a
Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/3391873 Change-Id: I05b367b4a56669c176bdcdfacf3faae5c88f3d78 Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
2 parents ebefb6b + c8ca54a commit cbec8f7

2 files changed

Lines changed: 40 additions & 26 deletions

File tree

services/surfaceflinger/SurfaceFlinger.cpp

Lines changed: 37 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4354,27 +4354,6 @@ void SurfaceFlinger::invalidateLayerStack(const ui::LayerFilter& layerFilter, co
43544354
status_t SurfaceFlinger::addClientLayer(LayerCreationArgs& args, const sp<IBinder>& handle,
43554355
const sp<Layer>& layer, const wp<Layer>& parent,
43564356
uint32_t* outTransformHint) {
4357-
if (mNumLayers >= MAX_LAYERS) {
4358-
static std::atomic<nsecs_t> lasttime{0};
4359-
nsecs_t now = systemTime();
4360-
if (lasttime != 0 && ns2s(now - lasttime.load()) < 10) {
4361-
ALOGE("AddClientLayer already dumped 10s before");
4362-
return NO_MEMORY;
4363-
} else {
4364-
lasttime = now;
4365-
}
4366-
4367-
ALOGE("AddClientLayer failed, mNumLayers (%zu) >= MAX_LAYERS (%zu)", mNumLayers.load(),
4368-
MAX_LAYERS);
4369-
static_cast<void>(mScheduler->schedule([&]() FTL_FAKE_GUARD(kMainThreadContext) {
4370-
ALOGE("Dumping on-screen layers.");
4371-
mLayerHierarchyBuilder.dumpLayerSample(mLayerHierarchyBuilder.getHierarchy());
4372-
ALOGE("Dumping off-screen layers.");
4373-
mLayerHierarchyBuilder.dumpLayerSample(mLayerHierarchyBuilder.getOffscreenHierarchy());
4374-
}));
4375-
return NO_MEMORY;
4376-
}
4377-
43784357
if (outTransformHint) {
43794358
*outTransformHint = mActiveDisplayTransformHint;
43804359
}
@@ -5145,16 +5124,15 @@ status_t SurfaceFlinger::mirrorDisplay(DisplayId displayId, const LayerCreationA
51455124
mirrorArgs.addToRoot = true;
51465125
mirrorArgs.layerStackToMirror = layerStack;
51475126
result = createEffectLayer(mirrorArgs, &outResult.handle, &rootMirrorLayer);
5127+
if (result != NO_ERROR) {
5128+
return result;
5129+
}
51485130
outResult.layerId = rootMirrorLayer->sequence;
51495131
outResult.layerName = String16(rootMirrorLayer->getDebugName());
5150-
result |= addClientLayer(mirrorArgs, outResult.handle, rootMirrorLayer /* layer */,
5132+
addClientLayer(mirrorArgs, outResult.handle, rootMirrorLayer /* layer */,
51515133
nullptr /* parent */, nullptr /* outTransformHint */);
51525134
}
51535135

5154-
if (result != NO_ERROR) {
5155-
return result;
5156-
}
5157-
51585136
setTransactionFlags(eTransactionFlushNeeded);
51595137
return NO_ERROR;
51605138
}
@@ -5172,6 +5150,9 @@ status_t SurfaceFlinger::createLayer(LayerCreationArgs& args, gui::CreateSurface
51725150
[[fallthrough]];
51735151
case ISurfaceComposerClient::eFXSurfaceEffect: {
51745152
result = createBufferStateLayer(args, &outResult.handle, &layer);
5153+
if (result != NO_ERROR) {
5154+
return result;
5155+
}
51755156
std::atomic<int32_t>* pendingBufferCounter = layer->getPendingBufferCounter();
51765157
if (pendingBufferCounter) {
51775158
std::string counterName = layer->getPendingBufferCounterName();
@@ -5211,18 +5192,48 @@ status_t SurfaceFlinger::createLayer(LayerCreationArgs& args, gui::CreateSurface
52115192

52125193
status_t SurfaceFlinger::createBufferStateLayer(LayerCreationArgs& args, sp<IBinder>* handle,
52135194
sp<Layer>* outLayer) {
5195+
if (checkLayerLeaks() != NO_ERROR) {
5196+
return NO_MEMORY;
5197+
}
52145198
*outLayer = getFactory().createBufferStateLayer(args);
52155199
*handle = (*outLayer)->getHandle();
52165200
return NO_ERROR;
52175201
}
52185202

52195203
status_t SurfaceFlinger::createEffectLayer(const LayerCreationArgs& args, sp<IBinder>* handle,
52205204
sp<Layer>* outLayer) {
5205+
if (checkLayerLeaks() != NO_ERROR) {
5206+
return NO_MEMORY;
5207+
}
52215208
*outLayer = getFactory().createEffectLayer(args);
52225209
*handle = (*outLayer)->getHandle();
52235210
return NO_ERROR;
52245211
}
52255212

5213+
status_t SurfaceFlinger::checkLayerLeaks() {
5214+
if (mNumLayers >= MAX_LAYERS) {
5215+
static std::atomic<nsecs_t> lasttime{0};
5216+
nsecs_t now = systemTime();
5217+
if (lasttime != 0 && ns2s(now - lasttime.load()) < 10) {
5218+
ALOGE("CreateLayer already dumped 10s before");
5219+
return NO_MEMORY;
5220+
} else {
5221+
lasttime = now;
5222+
}
5223+
5224+
ALOGE("CreateLayer failed, mNumLayers (%zu) >= MAX_LAYERS (%zu)", mNumLayers.load(),
5225+
MAX_LAYERS);
5226+
static_cast<void>(mScheduler->schedule([&]() FTL_FAKE_GUARD(kMainThreadContext) {
5227+
ALOGE("Dumping on-screen layers.");
5228+
mLayerHierarchyBuilder.dumpLayerSample(mLayerHierarchyBuilder.getHierarchy());
5229+
ALOGE("Dumping off-screen layers.");
5230+
mLayerHierarchyBuilder.dumpLayerSample(mLayerHierarchyBuilder.getOffscreenHierarchy());
5231+
}));
5232+
return NO_MEMORY;
5233+
}
5234+
return NO_ERROR;
5235+
}
5236+
52265237
void SurfaceFlinger::onHandleDestroyed(BBinder* handle, sp<Layer>& layer, uint32_t layerId) {
52275238
{
52285239
// Used to remove stalled transactions which uses an internal lock.

services/surfaceflinger/SurfaceFlinger.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -835,6 +835,9 @@ class SurfaceFlinger : public BnSurfaceComposer,
835835
status_t createEffectLayer(const LayerCreationArgs& args, sp<IBinder>* outHandle,
836836
sp<Layer>* outLayer);
837837

838+
// Checks if there are layer leaks before creating layer
839+
status_t checkLayerLeaks();
840+
838841
status_t mirrorLayer(const LayerCreationArgs& args, const sp<IBinder>& mirrorFromHandle,
839842
gui::CreateSurfaceResult& outResult);
840843

0 commit comments

Comments
 (0)