diff options
author | Mathias Agopian <mathias@google.com> | 2009-06-19 17:00:27 -0700 |
---|---|---|
committer | Mathias Agopian <mathias@google.com> | 2009-06-19 17:00:27 -0700 |
commit | 6edf5af578c1ab1fcd44b7c08ca371456e4b7430 (patch) | |
tree | 28ae8dec57019f5aac653b1a2d4df5cd061ee976 | |
parent | c8fb5b1979da4829e1486e6a1008c06c979b94b0 (diff) | |
download | frameworks_base-6edf5af578c1ab1fcd44b7c08ca371456e4b7430.zip frameworks_base-6edf5af578c1ab1fcd44b7c08ca371456e4b7430.tar.gz frameworks_base-6edf5af578c1ab1fcd44b7c08ca371456e4b7430.tar.bz2 |
fix a memory corruption where a SF Client could be used after it's been destroyed
-rw-r--r-- | libs/surfaceflinger/Layer.cpp | 21 | ||||
-rw-r--r-- | libs/surfaceflinger/Layer.h | 6 | ||||
-rw-r--r-- | libs/surfaceflinger/LayerBase.cpp | 18 | ||||
-rw-r--r-- | libs/surfaceflinger/LayerBase.h | 4 | ||||
-rw-r--r-- | libs/surfaceflinger/LayerBlur.cpp | 2 | ||||
-rw-r--r-- | libs/surfaceflinger/LayerBlur.h | 2 | ||||
-rw-r--r-- | libs/surfaceflinger/LayerBuffer.cpp | 2 | ||||
-rw-r--r-- | libs/surfaceflinger/LayerBuffer.h | 2 | ||||
-rw-r--r-- | libs/surfaceflinger/LayerDim.cpp | 2 | ||||
-rw-r--r-- | libs/surfaceflinger/LayerDim.h | 2 | ||||
-rw-r--r-- | libs/surfaceflinger/SurfaceFlinger.cpp | 75 | ||||
-rw-r--r-- | libs/surfaceflinger/SurfaceFlinger.h | 20 |
12 files changed, 85 insertions, 71 deletions
diff --git a/libs/surfaceflinger/Layer.cpp b/libs/surfaceflinger/Layer.cpp index 5b260c4..218cb39 100644 --- a/libs/surfaceflinger/Layer.cpp +++ b/libs/surfaceflinger/Layer.cpp @@ -47,7 +47,7 @@ const char* const Layer::typeID = "Layer"; // --------------------------------------------------------------------------- -Layer::Layer(SurfaceFlinger* flinger, DisplayID display, Client* c, int32_t i) +Layer::Layer(SurfaceFlinger* flinger, DisplayID display, const sp<Client>& c, int32_t i) : LayerBaseClient(flinger, display, c, i), mSecure(false), mFrontBufferIndex(1), @@ -100,8 +100,7 @@ status_t Layer::ditch() return NO_ERROR; } -status_t Layer::setBuffers( Client* client, - uint32_t w, uint32_t h, +status_t Layer::setBuffers( uint32_t w, uint32_t h, PixelFormat format, uint32_t flags) { PixelFormatInfo info; @@ -285,6 +284,14 @@ sp<SurfaceBuffer> Layer::peekBuffer() return buffer; } +void Layer::scheduleBroadcast() +{ + sp<Client> ourClient(client.promote()); + if (ourClient != 0) { + mFlinger->scheduleBroadcast(ourClient); + } +} + uint32_t Layer::doTransaction(uint32_t flags) { const Layer::State& front(drawingState()); @@ -380,7 +387,7 @@ uint32_t Layer::doTransaction(uint32_t flags) eResizeBuffer1 : eResizeBuffer0; android_atomic_and(~mask, &(lcblk->swapState)); // since a buffer became available, we can let the client go... - mFlinger->scheduleBroadcast(client); + scheduleBroadcast(); mResizeTransactionDone = true; // we're being resized and there is a freeze display request, @@ -489,7 +496,7 @@ void Layer::lockPageFlip(bool& recomputeVisibleRegions) if (UNLIKELY(state & eInvalidSurface)) { // if eInvalidSurface is set, this means the surface // became invalid during a transaction (NO_MEMORY for instance) - mFlinger->scheduleBroadcast(client); + scheduleBroadcast(); return; } @@ -627,7 +634,7 @@ void Layer::unlockPageFlip( // client could be blocked, so signal them so they get a // chance to reevaluate their condition. - mFlinger->scheduleBroadcast(client); + scheduleBroadcast(); } } @@ -638,7 +645,7 @@ void Layer::finishPageFlip() "layer %p wasn't locked!", this); android_atomic_and(~eBusy, &(lcblk->swapState)); } - mFlinger->scheduleBroadcast(client); + scheduleBroadcast(); } // --------------------------------------------------------------------------- diff --git a/libs/surfaceflinger/Layer.h b/libs/surfaceflinger/Layer.h index e16d9f4..a19c171 100644 --- a/libs/surfaceflinger/Layer.h +++ b/libs/surfaceflinger/Layer.h @@ -58,7 +58,7 @@ public: virtual uint32_t getTypeInfo() const { return typeInfo; } Layer(SurfaceFlinger* flinger, DisplayID display, - Client* c, int32_t i); + const sp<Client>& client, int32_t i); virtual ~Layer(); @@ -66,8 +66,7 @@ public: return frontBuffer().getPixelFormat(); } - status_t setBuffers( Client* client, - uint32_t w, uint32_t h, + status_t setBuffers( uint32_t w, uint32_t h, PixelFormat format, uint32_t flags=0); virtual void onDraw(const Region& clip) const; @@ -105,6 +104,7 @@ private: Region post(uint32_t* oldState, bool& recomputeVisibleRegions); sp<SurfaceBuffer> peekBuffer(); void destroy(); + void scheduleBroadcast(); class SurfaceLayer : public LayerBaseClient::Surface diff --git a/libs/surfaceflinger/LayerBase.cpp b/libs/surfaceflinger/LayerBase.cpp index b65c983..3b86350 100644 --- a/libs/surfaceflinger/LayerBase.cpp +++ b/libs/surfaceflinger/LayerBase.cpp @@ -640,16 +640,17 @@ regular: // --------------------------------------------------------------------------- LayerBaseClient::LayerBaseClient(SurfaceFlinger* flinger, DisplayID display, - Client* c, int32_t i) - : LayerBase(flinger, display), client(c), - lcblk( c ? &(c->ctrlblk->layers[i]) : 0 ), + const sp<Client>& client, int32_t i) + : LayerBase(flinger, display), client(client), + lcblk( client!=0 ? &(client->ctrlblk->layers[i]) : 0 ), mIndex(i) { } void LayerBaseClient::onFirstRef() { - if (client) { + sp<Client> client(this->client.promote()); + if (client != 0) { client->bindLayer(this, mIndex); // Initialize this layer's control block memset(this->lcblk, 0, sizeof(layer_cblk_t)); @@ -661,13 +662,16 @@ void LayerBaseClient::onFirstRef() LayerBaseClient::~LayerBaseClient() { - if (client) { + sp<Client> client(this->client.promote()); + if (client != 0) { client->free(mIndex); } } -int32_t LayerBaseClient::serverIndex() const { - if (client) { +int32_t LayerBaseClient::serverIndex() const +{ + sp<Client> client(this->client.promote()); + if (client != 0) { return (client->cid<<16)|mIndex; } return 0xFFFF0000 | mIndex; diff --git a/libs/surfaceflinger/LayerBase.h b/libs/surfaceflinger/LayerBase.h index 509dedd..b5265d2 100644 --- a/libs/surfaceflinger/LayerBase.h +++ b/libs/surfaceflinger/LayerBase.h @@ -297,11 +297,11 @@ public: virtual uint32_t getTypeInfo() const { return typeInfo; } LayerBaseClient(SurfaceFlinger* flinger, DisplayID display, - Client* client, int32_t i); + const sp<Client>& client, int32_t i); virtual ~LayerBaseClient(); virtual void onFirstRef(); - Client* const client; + wp<Client> client; layer_cblk_t* const lcblk; inline uint32_t getIdentity() const { return mIdentity; } diff --git a/libs/surfaceflinger/LayerBlur.cpp b/libs/surfaceflinger/LayerBlur.cpp index 3d22e3a..00abd5a 100644 --- a/libs/surfaceflinger/LayerBlur.cpp +++ b/libs/surfaceflinger/LayerBlur.cpp @@ -38,7 +38,7 @@ const char* const LayerBlur::typeID = "LayerBlur"; // --------------------------------------------------------------------------- LayerBlur::LayerBlur(SurfaceFlinger* flinger, DisplayID display, - Client* client, int32_t i) + const sp<Client>& client, int32_t i) : LayerBaseClient(flinger, display, client, i), mCacheDirty(true), mRefreshCache(true), mCacheAge(0), mTextureName(-1U) { diff --git a/libs/surfaceflinger/LayerBlur.h b/libs/surfaceflinger/LayerBlur.h index 24b1156..0c3e6eb 100644 --- a/libs/surfaceflinger/LayerBlur.h +++ b/libs/surfaceflinger/LayerBlur.h @@ -39,7 +39,7 @@ public: virtual uint32_t getTypeInfo() const { return typeInfo; } LayerBlur(SurfaceFlinger* flinger, DisplayID display, - Client* client, int32_t i); + const sp<Client>& client, int32_t i); virtual ~LayerBlur(); virtual void onDraw(const Region& clip) const; diff --git a/libs/surfaceflinger/LayerBuffer.cpp b/libs/surfaceflinger/LayerBuffer.cpp index 8be91c9..02b3651 100644 --- a/libs/surfaceflinger/LayerBuffer.cpp +++ b/libs/surfaceflinger/LayerBuffer.cpp @@ -40,7 +40,7 @@ const char* const LayerBuffer::typeID = "LayerBuffer"; // --------------------------------------------------------------------------- LayerBuffer::LayerBuffer(SurfaceFlinger* flinger, DisplayID display, - Client* client, int32_t i) + const sp<Client>& client, int32_t i) : LayerBaseClient(flinger, display, client, i), mNeedsBlending(false) { diff --git a/libs/surfaceflinger/LayerBuffer.h b/libs/surfaceflinger/LayerBuffer.h index 22af43e..587d31b 100644 --- a/libs/surfaceflinger/LayerBuffer.h +++ b/libs/surfaceflinger/LayerBuffer.h @@ -57,7 +57,7 @@ public: virtual uint32_t getTypeInfo() const { return typeInfo; } LayerBuffer(SurfaceFlinger* flinger, DisplayID display, - Client* client, int32_t i); + const sp<Client>& client, int32_t i); virtual ~LayerBuffer(); virtual void onFirstRef(); diff --git a/libs/surfaceflinger/LayerDim.cpp b/libs/surfaceflinger/LayerDim.cpp index b95e184..5cb883c 100644 --- a/libs/surfaceflinger/LayerDim.cpp +++ b/libs/surfaceflinger/LayerDim.cpp @@ -40,7 +40,7 @@ int32_t LayerDim::sHeight; // --------------------------------------------------------------------------- LayerDim::LayerDim(SurfaceFlinger* flinger, DisplayID display, - Client* client, int32_t i) + const sp<Client>& client, int32_t i) : LayerBaseClient(flinger, display, client, i) { } diff --git a/libs/surfaceflinger/LayerDim.h b/libs/surfaceflinger/LayerDim.h index d9b70b4..33bd49d 100644 --- a/libs/surfaceflinger/LayerDim.h +++ b/libs/surfaceflinger/LayerDim.h @@ -44,7 +44,7 @@ public: virtual uint32_t getTypeInfo() const { return typeInfo; } LayerDim(SurfaceFlinger* flinger, DisplayID display, - Client* client, int32_t i); + const sp<Client>& client, int32_t i); virtual ~LayerDim(); virtual void onDraw(const Region& clip) const; diff --git a/libs/surfaceflinger/SurfaceFlinger.cpp b/libs/surfaceflinger/SurfaceFlinger.cpp index 90e0bb5..2e7bbea 100644 --- a/libs/surfaceflinger/SurfaceFlinger.cpp +++ b/libs/surfaceflinger/SurfaceFlinger.cpp @@ -227,14 +227,13 @@ sp<ISurfaceFlingerClient> SurfaceFlinger::createConnection() Mutex::Autolock _l(mStateLock); uint32_t token = mTokens.acquire(); - Client* client = new Client(token, this); - if ((client == 0) || (client->ctrlblk == 0)) { + sp<Client> client = new Client(token, this); + if (client->ctrlblk == 0) { mTokens.release(token); return 0; } status_t err = mClientsMap.add(token, client); if (err < 0) { - delete client; mTokens.release(token); return 0; } @@ -246,8 +245,8 @@ sp<ISurfaceFlingerClient> SurfaceFlinger::createConnection() void SurfaceFlinger::destroyConnection(ClientID cid) { Mutex::Autolock _l(mStateLock); - Client* const client = mClientsMap.valueFor(cid); - if (client) { + sp<Client> client = mClientsMap.valueFor(cid); + if (client != 0) { // free all the layers this client owns Vector< wp<LayerBaseClient> > layers(client->getLayers()); const size_t count = layers.size(); @@ -876,7 +875,7 @@ void SurfaceFlinger::unlockClients() } } -void SurfaceFlinger::scheduleBroadcast(Client* client) +void SurfaceFlinger::scheduleBroadcast(const sp<Client>& client) { if (mLastScheduledBroadcast != client) { mLastScheduledBroadcast = client; @@ -886,19 +885,22 @@ void SurfaceFlinger::scheduleBroadcast(Client* client) void SurfaceFlinger::executeScheduledBroadcasts() { - SortedVector<Client*>& list = mScheduledBroadcasts; + SortedVector< wp<Client> >& list(mScheduledBroadcasts); size_t count = list.size(); while (count--) { - per_client_cblk_t* const cblk = list[count]->ctrlblk; - if (cblk->lock.tryLock() == NO_ERROR) { - cblk->cv.broadcast(); - list.removeAt(count); - cblk->lock.unlock(); - } else { - // schedule another round - LOGW("executeScheduledBroadcasts() skipped, " - "contention on the client. We'll try again later..."); - signalDelayedEvent(ms2ns(4)); + sp<Client> client = list[count].promote(); + if (client != 0) { + per_client_cblk_t* const cblk = client->ctrlblk; + if (cblk->lock.tryLock() == NO_ERROR) { + cblk->cv.broadcast(); + list.removeAt(count); + cblk->lock.unlock(); + } else { + // schedule another round + LOGW("executeScheduledBroadcasts() skipped, " + "contention on the client. We'll try again later..."); + signalDelayedEvent(ms2ns(4)); + } } } mLastScheduledBroadcast = 0; @@ -1090,11 +1092,11 @@ void SurfaceFlinger::free_resources_l() mLayersRemoved = false; // free resources associated with disconnected clients - SortedVector<Client*>& scheduledBroadcasts(mScheduledBroadcasts); - Vector<Client*>& disconnectedClients(mDisconnectedClients); + SortedVector< wp<Client> >& scheduledBroadcasts(mScheduledBroadcasts); + Vector< sp<Client> >& disconnectedClients(mDisconnectedClients); const size_t count = disconnectedClients.size(); for (size_t i=0 ; i<count ; i++) { - Client* client = disconnectedClients[i]; + sp<Client> client = disconnectedClients[i]; // if this client is the scheduled broadcast list, // remove it from there (and we don't need to signal it // since it is dead). @@ -1103,7 +1105,6 @@ void SurfaceFlinger::free_resources_l() scheduledBroadcasts.removeItemsAt(index); } mTokens.release(client->cid); - delete client; } disconnectedClients.clear(); } @@ -1194,14 +1195,14 @@ sp<ISurface> SurfaceFlinger::createSurface(ClientID clientId, int pid, sp<LayerBaseClient> layer; sp<LayerBaseClient::Surface> surfaceHandle; Mutex::Autolock _l(mStateLock); - Client* const c = mClientsMap.valueFor(clientId); - if (UNLIKELY(!c)) { + sp<Client> client = mClientsMap.valueFor(clientId); + if (UNLIKELY(client == 0)) { LOGE("createSurface() failed, client not found (id=%d)", clientId); return surfaceHandle; } //LOGD("createSurface for pid %d (%d x %d)", pid, w, h); - int32_t id = c->generateId(pid); + int32_t id = client->generateId(pid); if (uint32_t(id) >= NUM_LAYERS_MAX) { LOGE("createSurface() failed, generateId = %d", id); return surfaceHandle; @@ -1210,16 +1211,16 @@ sp<ISurface> SurfaceFlinger::createSurface(ClientID clientId, int pid, switch (flags & eFXSurfaceMask) { case eFXSurfaceNormal: if (UNLIKELY(flags & ePushBuffers)) { - layer = createPushBuffersSurfaceLocked(c, d, id, w, h, flags); + layer = createPushBuffersSurfaceLocked(client, d, id, w, h, flags); } else { - layer = createNormalSurfaceLocked(c, d, id, w, h, format, flags); + layer = createNormalSurfaceLocked(client, d, id, w, h, format, flags); } break; case eFXSurfaceBlur: - layer = createBlurSurfaceLocked(c, d, id, w, h, flags); + layer = createBlurSurfaceLocked(client, d, id, w, h, flags); break; case eFXSurfaceDim: - layer = createDimSurfaceLocked(c, d, id, w, h, flags); + layer = createDimSurfaceLocked(client, d, id, w, h, flags); break; } @@ -1234,7 +1235,7 @@ sp<ISurface> SurfaceFlinger::createSurface(ClientID clientId, int pid, } sp<LayerBaseClient> SurfaceFlinger::createNormalSurfaceLocked( - Client* client, DisplayID display, + const sp<Client>& client, DisplayID display, int32_t id, uint32_t w, uint32_t h, PixelFormat format, uint32_t flags) { // initialize the surfaces @@ -1249,7 +1250,7 @@ sp<LayerBaseClient> SurfaceFlinger::createNormalSurfaceLocked( } sp<Layer> layer = new Layer(this, display, client, id); - status_t err = layer->setBuffers(client, w, h, format, flags); + status_t err = layer->setBuffers(w, h, format, flags); if (LIKELY(err == NO_ERROR)) { layer->initStates(w, h, flags); addLayer_l(layer); @@ -1261,7 +1262,7 @@ sp<LayerBaseClient> SurfaceFlinger::createNormalSurfaceLocked( } sp<LayerBaseClient> SurfaceFlinger::createBlurSurfaceLocked( - Client* client, DisplayID display, + const sp<Client>& client, DisplayID display, int32_t id, uint32_t w, uint32_t h, uint32_t flags) { sp<LayerBlur> layer = new LayerBlur(this, display, client, id); @@ -1271,7 +1272,7 @@ sp<LayerBaseClient> SurfaceFlinger::createBlurSurfaceLocked( } sp<LayerBaseClient> SurfaceFlinger::createDimSurfaceLocked( - Client* client, DisplayID display, + const sp<Client>& client, DisplayID display, int32_t id, uint32_t w, uint32_t h, uint32_t flags) { sp<LayerDim> layer = new LayerDim(this, display, client, id); @@ -1281,7 +1282,7 @@ sp<LayerBaseClient> SurfaceFlinger::createDimSurfaceLocked( } sp<LayerBaseClient> SurfaceFlinger::createPushBuffersSurfaceLocked( - Client* client, DisplayID display, + const sp<Client>& client, DisplayID display, int32_t id, uint32_t w, uint32_t h, uint32_t flags) { sp<LayerBuffer> layer = new LayerBuffer(this, display, client, id); @@ -1446,7 +1447,7 @@ status_t SurfaceFlinger::dump(int fd, const Vector<String16>& args) size_t s = mClientsMap.size(); char name[64]; for (size_t i=0 ; i<s ; i++) { - Client* client = mClientsMap.valueAt(i); + sp<Client> client = mClientsMap.valueAt(i); sprintf(name, " Client (id=0x%08x)", client->cid); client->dump(name); } @@ -1474,10 +1475,11 @@ status_t SurfaceFlinger::dump(int fd, const Vector<String16>& args) sp<LayerBaseClient> lbc = LayerBase::dynamicCast< LayerBaseClient* >(layer.get()); if (lbc != 0) { + sp<Client> client(lbc->client.promote()); snprintf(buffer, SIZE, " " "id=0x%08x, client=0x%08x, identity=%u\n", - lbc->clientIndex(), lbc->client ? lbc->client->cid : 0, + lbc->clientIndex(), client.get() ? client->cid : 0, lbc->getIdentity()); } result.append(buffer); @@ -1512,7 +1514,8 @@ status_t SurfaceFlinger::dump(int fd, const Vector<String16>& args) mFreezeDisplay?"yes":"no", mFreezeCount, mCurrentState.orientation, hw.canDraw()); result.append(buffer); - snprintf(buffer, SIZE, " purgatory size: %d\n", mLayerPurgatory.size()); + snprintf(buffer, SIZE, " purgatory size: %d, client count: %d\n", + mLayerPurgatory.size(), mClientsMap.size()); result.append(buffer); const BufferAllocator& alloc(BufferAllocator::get()); alloc.dump(result); diff --git a/libs/surfaceflinger/SurfaceFlinger.h b/libs/surfaceflinger/SurfaceFlinger.h index ac13340..149eef0 100644 --- a/libs/surfaceflinger/SurfaceFlinger.h +++ b/libs/surfaceflinger/SurfaceFlinger.h @@ -64,7 +64,7 @@ typedef int32_t ClientID; // --------------------------------------------------------------------------- -class Client +class Client : public RefBase { public: Client(ClientID cid, const sp<SurfaceFlinger>& flinger); @@ -188,20 +188,20 @@ private: uint32_t flags); sp<LayerBaseClient> createNormalSurfaceLocked( - Client* client, DisplayID display, + const sp<Client>& client, DisplayID display, int32_t id, uint32_t w, uint32_t h, PixelFormat format, uint32_t flags); sp<LayerBaseClient> createBlurSurfaceLocked( - Client* client, DisplayID display, + const sp<Client>& client, DisplayID display, int32_t id, uint32_t w, uint32_t h, uint32_t flags); sp<LayerBaseClient> createDimSurfaceLocked( - Client* client, DisplayID display, + const sp<Client>& client, DisplayID display, int32_t id, uint32_t w, uint32_t h, uint32_t flags); sp<LayerBaseClient> createPushBuffersSurfaceLocked( - Client* client, DisplayID display, + const sp<Client>& client, DisplayID display, int32_t id, uint32_t w, uint32_t h, uint32_t flags); status_t removeSurface(SurfaceID surface_id); @@ -262,7 +262,7 @@ private: bool lockPageFlip(const LayerVector& currentLayers); void unlockPageFlip(const LayerVector& currentLayers); void handleRepaint(); - void scheduleBroadcast(Client* client); + void scheduleBroadcast(const sp<Client>& client); void executeScheduledBroadcasts(); void postFramebuffer(); void composeSurfaces(const Region& dirty); @@ -312,11 +312,11 @@ private: // protected by mStateLock (but we could use another lock) Tokenizer mTokens; - DefaultKeyedVector<ClientID, Client*> mClientsMap; + DefaultKeyedVector<ClientID, sp<Client> > mClientsMap; DefaultKeyedVector<SurfaceID, sp<LayerBaseClient> > mLayerMap; GraphicPlane mGraphicPlanes[1]; bool mLayersRemoved; - Vector<Client*> mDisconnectedClients; + Vector< sp<Client> > mDisconnectedClients; // constant members (no synchronization needed for access) sp<MemoryDealer> mServerHeap; @@ -333,8 +333,8 @@ private: Region mDirtyRegion; Region mInvalidRegion; Region mWormholeRegion; - Client* mLastScheduledBroadcast; - SortedVector<Client*> mScheduledBroadcasts; + wp<Client> mLastScheduledBroadcast; + SortedVector< wp<Client> > mScheduledBroadcasts; bool mVisibleRegionsDirty; bool mDeferReleaseConsole; bool mFreezeDisplay; |