diff options
author | Mathias Agopian <mathias@google.com> | 2011-05-03 16:21:41 -0700 |
---|---|---|
committer | Mathias Agopian <mathias@google.com> | 2011-05-23 16:12:02 -0700 |
commit | a725ccda9e0e9aec80f9f0b1a63d94d46a13c4b4 (patch) | |
tree | b3cdee9690b7b4556d6b120692a4c673cdae3a60 | |
parent | 4e4ad34159dc1c720a3e6145bb9f14c60fb1a3f7 (diff) | |
download | base-a725ccda9e0e9aec80f9f0b1a63d94d46a13c4b4.tar.gz |
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.
Bug: 4483046
Change-Id: I5262bf1124d9a65ec6f8ffd8e367356fc33a7536
-rw-r--r-- | services/surfaceflinger/SurfaceFlinger.cpp | 20 | ||||
-rw-r--r-- | services/surfaceflinger/SurfaceFlinger.h | 12 |
2 files changed, 21 insertions, 11 deletions
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index c08e2c941235..9f007b314d40 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1030,15 +1030,15 @@ status_t SurfaceFlinger::addLayer_l(const sp<LayerBase>& layer) 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) @@ -2253,15 +2253,17 @@ status_t Client::initCheck() const { 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++) { @@ -2271,9 +2273,11 @@ void Client::detachLayer(const LayerBaseClient* layer) } } } -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)); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index df1ca4892c9d..bb89f4314fd4 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -66,7 +66,7 @@ public: status_t initCheck() const; // protected by SurfaceFlinger::mStateLock - ssize_t attachLayer(const sp<LayerBaseClient>& layer); + size_t attachLayer(const sp<LayerBaseClient>& layer); void detachLayer(const LayerBaseClient* layer); sp<LayerBaseClient> getLayerUser(int32_t i) const; @@ -82,9 +82,15 @@ private: virtual status_t destroySurface(SurfaceID surfaceId); virtual status_t setState(int32_t count, const layer_state_t* states); - DefaultKeyedVector< size_t, wp<LayerBaseClient> > mLayers; + // constant sp<SurfaceFlinger> mFlinger; - int32_t mNameGenerator; + + // protected by mLock + DefaultKeyedVector< size_t, wp<LayerBaseClient> > mLayers; + size_t mNameGenerator; + + // thread-safe + mutable Mutex mLock; }; class UserClient : public BnSurfaceComposerClient |