Merge changes from topic 'SF: Fix a couple of Layer ref count issues'
* changes:
SF: Change wp usage for deferred transactions in the interceptor to sp
SF: Fix a couple of Layer ref count issues
diff --git a/services/surfaceflinger/Client.cpp b/services/surfaceflinger/Client.cpp
index 415bdca..e14a59b 100644
--- a/services/surfaceflinger/Client.cpp
+++ b/services/surfaceflinger/Client.cpp
@@ -43,10 +43,7 @@
{
const size_t count = mLayers.size();
for (size_t i=0 ; i<count ; i++) {
- sp<Layer> layer(mLayers.valueAt(i).promote());
- if (layer != 0) {
- mFlinger->removeLayer(layer);
- }
+ mFlinger->removeLayer(mLayers.valueAt(i));
}
}
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 6c1234f..c8911f3 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -1313,9 +1313,14 @@
// If this transaction is waiting on the receipt of a frame, generate a sync
// point and send it to the remote layer.
if (mCurrentState.handle != nullptr) {
- sp<Handle> handle = static_cast<Handle*>(mCurrentState.handle.get());
- sp<Layer> handleLayer = handle->owner.promote();
- if (handleLayer == nullptr) {
+ sp<IBinder> strongBinder = mCurrentState.handle.promote();
+ sp<Handle> handle = nullptr;
+ sp<Layer> handleLayer = nullptr;
+ if (strongBinder != nullptr) {
+ handle = static_cast<Handle*>(strongBinder.get());
+ handleLayer = handle->owner.promote();
+ }
+ if (strongBinder == nullptr || handleLayer == nullptr) {
ALOGE("[%s] Unable to promote Layer handle", mName.string());
// If we can't promote the layer we are intended to wait on,
// then it is expired or otherwise invalid. Allow this transaction
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index a63656e..56f6607 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -128,7 +128,7 @@
// If set, defers this state update until the Layer identified by handle
// receives a frame with the given frameNumber
- sp<IBinder> handle;
+ wp<IBinder> handle;
uint64_t frameNumber;
// the transparentRegion hint is a bit special, it's latched only
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 5ce63a3..c15b4bc 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2283,8 +2283,14 @@
return NO_ERROR;
}
-status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer) {
+status_t SurfaceFlinger::removeLayer(const wp<Layer>& weakLayer) {
Mutex::Autolock _l(mStateLock);
+ sp<Layer> layer = weakLayer.promote();
+ if (layer == nullptr) {
+ // The layer has already been removed, carry on
+ return NO_ERROR;
+ }
+
ssize_t index = mCurrentState.layersSortedByZ.remove(layer);
if (index >= 0) {
mLayersPendingRemoval.push(layer);
@@ -2630,14 +2636,7 @@
{
// called by ~LayerCleaner() when all references to the IBinder (handle)
// are gone
- status_t err = NO_ERROR;
- sp<Layer> l(layer.promote());
- if (l != NULL) {
- err = removeLayer(l);
- ALOGE_IF(err<0 && err != NAME_NOT_FOUND,
- "error removing layer=%p (%s)", l.get(), strerror(-err));
- }
- return err;
+ return removeLayer(layer);
}
// ---------------------------------------------------------------------------
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 838eaa7..6449e57 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -314,7 +314,7 @@
status_t onLayerDestroyed(const wp<Layer>& layer);
// remove a layer from SurfaceFlinger immediately
- status_t removeLayer(const sp<Layer>& layer);
+ status_t removeLayer(const wp<Layer>& layer);
// add a layer to SurfaceFlinger
status_t addClientLayer(const sp<Client>& client,
diff --git a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
index c88c0c4..97ad9b2 100644
--- a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
+++ b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
@@ -2199,8 +2199,14 @@
return NO_ERROR;
}
-status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer) {
+status_t SurfaceFlinger::removeLayer(const wp<Layer>& weakLayer) {
Mutex::Autolock _l(mStateLock);
+ sp<Layer> layer = weakLayer.promote();
+ if (layer == nullptr) {
+ // The layer has already been removed, carry on
+ return NO_ERROR;
+ }
+
ssize_t index = mCurrentState.layersSortedByZ.remove(layer);
if (index >= 0) {
mLayersPendingRemoval.push(layer);
@@ -2546,14 +2552,7 @@
{
// called by ~LayerCleaner() when all references to the IBinder (handle)
// are gone
- status_t err = NO_ERROR;
- sp<Layer> l(layer.promote());
- if (l != NULL) {
- err = removeLayer(l);
- ALOGE_IF(err<0 && err != NAME_NOT_FOUND,
- "error removing layer=%p (%s)", l.get(), strerror(-err));
- }
- return err;
+ return removeLayer(layer);
}
// ---------------------------------------------------------------------------
diff --git a/services/surfaceflinger/SurfaceInterceptor.cpp b/services/surfaceflinger/SurfaceInterceptor.cpp
index 594b03a..4ae3580 100644
--- a/services/surfaceflinger/SurfaceInterceptor.cpp
+++ b/services/surfaceflinger/SurfaceInterceptor.cpp
@@ -130,7 +130,8 @@
return NO_ERROR;
}
-const sp<const Layer> SurfaceInterceptor::getLayer(const sp<const IBinder>& handle) {
+const sp<const Layer> SurfaceInterceptor::getLayer(const wp<const IBinder>& weakHandle) {
+ const sp<const IBinder>& handle(weakHandle.promote());
const auto layerHandle(static_cast<const Layer::Handle*>(handle.get()));
const sp<const Layer> layer(layerHandle->owner.promote());
// layer could be a nullptr at this point
@@ -279,10 +280,10 @@
}
void SurfaceInterceptor::addDeferTransactionLocked(Transaction* transaction, int32_t layerId,
- const sp<const IBinder>& handle, uint64_t frameNumber)
+ const wp<const IBinder>& weakHandle, uint64_t frameNumber)
{
SurfaceChange* change(createSurfaceChangeLocked(transaction, layerId));
- const sp<const Layer> layer(getLayer(handle));
+ const sp<const Layer> layer(getLayer(weakHandle));
if (layer == nullptr) {
ALOGE("An existing layer could not be retrieved with the handle"
" for the deferred transaction");
diff --git a/services/surfaceflinger/SurfaceInterceptor.h b/services/surfaceflinger/SurfaceInterceptor.h
index 02d4288..4695138 100644
--- a/services/surfaceflinger/SurfaceInterceptor.h
+++ b/services/surfaceflinger/SurfaceInterceptor.h
@@ -70,7 +70,7 @@
void addInitialDisplayStateLocked(Increment* increment, const DisplayDeviceState& display);
status_t writeProtoFileLocked();
- const sp<const Layer> getLayer(const sp<const IBinder>& handle);
+ const sp<const Layer> getLayer(const wp<const IBinder>& weakHandle);
const std::string getLayerName(const sp<const Layer>& layer);
int32_t getLayerId(const sp<const Layer>& layer);
@@ -99,7 +99,7 @@
void addLayerStackLocked(Transaction* transaction, int32_t layerId, uint32_t layerStack);
void addCropLocked(Transaction* transaction, int32_t layerId, const Rect& rect);
void addDeferTransactionLocked(Transaction* transaction, int32_t layerId,
- const sp<const IBinder>& handle, uint64_t frameNumber);
+ const wp<const IBinder>& weakHandle, uint64_t frameNumber);
void addFinalCropLocked(Transaction* transaction, int32_t layerId, const Rect& rect);
void addOverrideScalingModeLocked(Transaction* transaction, int32_t layerId,
int32_t overrideScalingMode);