Fix a race-condtion in SurfaceFlinger that could lead to a crash.
Client::mLayers could be accessed from different threads.
On one side from Client::attachLayer() which is currently
called from a binder thread; on the other side from
Client::detachLayer() which is always called from the main
thread.
This could lead to a corruption of Client::mLayers.
We fix this issue by adding an internal lock to Client.
Change-Id: Ib1317d7750ed5030e6f577efe34b69fc10198bd3
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 7506f29..ad6ab8e 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1094,15 +1094,15 @@
ssize_t SurfaceFlinger::addClientLayer(const sp<Client>& client,
const sp<LayerBaseClient>& lbc)
{
- Mutex::Autolock _l(mStateLock);
-
// attach this layer to the client
- ssize_t name = client->attachLayer(lbc);
+ size_t name = client->attachLayer(lbc);
+
+ Mutex::Autolock _l(mStateLock);
// add this layer to the current state list
addLayer_l(lbc);
- return name;
+ return ssize_t(name);
}
status_t SurfaceFlinger::removeLayer(const sp<LayerBase>& layer)
@@ -2381,15 +2381,17 @@
return NO_ERROR;
}
-ssize_t Client::attachLayer(const sp<LayerBaseClient>& layer)
+size_t Client::attachLayer(const sp<LayerBaseClient>& layer)
{
- int32_t name = android_atomic_inc(&mNameGenerator);
+ Mutex::Autolock _l(mLock);
+ size_t name = mNameGenerator++;
mLayers.add(name, layer);
return name;
}
void Client::detachLayer(const LayerBaseClient* layer)
{
+ Mutex::Autolock _l(mLock);
// we do a linear search here, because this doesn't happen often
const size_t count = mLayers.size();
for (size_t i=0 ; i<count ; i++) {
@@ -2399,9 +2401,11 @@
}
}
}
-sp<LayerBaseClient> Client::getLayerUser(int32_t i) const {
+sp<LayerBaseClient> Client::getLayerUser(int32_t i) const
+{
+ Mutex::Autolock _l(mLock);
sp<LayerBaseClient> lbc;
- const wp<LayerBaseClient>& layer(mLayers.valueFor(i));
+ wp<LayerBaseClient> layer(mLayers.valueFor(i));
if (layer != 0) {
lbc = layer.promote();
LOGE_IF(lbc==0, "getLayerUser(name=%d) is dead", int(i));