diff --git a/include/private/surfaceflinger/SharedBufferStack.h b/include/private/surfaceflinger/SharedBufferStack.h index c11c85552..633b5436d 100644 --- a/include/private/surfaceflinger/SharedBufferStack.h +++ b/include/private/surfaceflinger/SharedBufferStack.h @@ -43,15 +43,6 @@ namespace android { * unless they are in use by the server, which is only the case for the last * dequeue-able buffer. When these various conditions are not met, the caller * waits until the condition is met. - * - * - * CAVEATS: - * - * In the current implementation there are several limitations: - * - buffers must be locked in the same order they've been dequeued - * - buffers must be enqueued in the same order they've been locked - * - dequeue() is not reentrant - * - no error checks are done on the condition above * */ @@ -269,7 +260,9 @@ private: // ---------------------------------------------------------------------------- -class SharedBufferServer : public SharedBufferBase +class SharedBufferServer + : public SharedBufferBase, + public LightRefBase { public: SharedBufferServer(SharedClient* sharedClient, int surface, int num, @@ -290,6 +283,9 @@ public: private: + friend class LightRefBase; + ~SharedBufferServer(); + /* * BufferList is basically a fixed-capacity sorted-vector of * unsigned 5-bits ints using a 32-bits int as storage. diff --git a/include/surfaceflinger/Surface.h b/include/surfaceflinger/Surface.h index ac01ce57c..f3339117d 100644 --- a/include/surfaceflinger/Surface.h +++ b/include/surfaceflinger/Surface.h @@ -60,7 +60,6 @@ public: static bool isSameSurface( const sp& lhs, const sp& rhs); - SurfaceID ID() const { return mToken; } uint32_t getFlags() const { return mFlags; } uint32_t getIdentity() const { return mIdentity; } @@ -145,6 +144,9 @@ public: uint32_t reserved[2]; }; + static status_t writeToParcel( + const sp& control, Parcel* parcel); + static sp readFromParcel( const Parcel& data, const sp& other); @@ -153,7 +155,6 @@ public: } bool isValid(); - SurfaceID ID() const { return mToken; } uint32_t getFlags() const { return mFlags; } uint32_t getIdentity() const { return mIdentity; } @@ -267,7 +268,6 @@ private: SharedBufferClient* mSharedBufferClient; status_t mInitCheck; sp mSurface; - SurfaceID mToken; uint32_t mIdentity; PixelFormat mFormat; uint32_t mFlags; diff --git a/libs/surfaceflinger/Layer.cpp b/libs/surfaceflinger/Layer.cpp index a94fdd4e6..e7247bd90 100644 --- a/libs/surfaceflinger/Layer.cpp +++ b/libs/surfaceflinger/Layer.cpp @@ -76,15 +76,18 @@ Layer::~Layer() status_t Layer::setToken(const sp& userClient, SharedClient* sharedClient, int32_t token) { - SharedBufferServer* lcblk = new SharedBufferServer( + sp lcblk = new SharedBufferServer( sharedClient, token, mBufferManager.getDefaultBufferCount(), getIdentity()); status_t err = mUserClientRef.setToken(userClient, lcblk, token); - if (err != NO_ERROR) { - LOGE("ClientRef::setToken(%p, %p, %u) failed", - userClient.get(), lcblk, token); - delete lcblk; + + LOGE_IF(err != NO_ERROR, + "ClientRef::setToken(%p, %p, %u) failed", + userClient.get(), lcblk.get(), token); + + if (err == NO_ERROR) { + // we need to free the buffers associated with this surface } return err; @@ -95,6 +98,11 @@ int32_t Layer::getToken() const return mUserClientRef.getToken(); } +sp Layer::getClient() const +{ + return mUserClientRef.getClient(); +} + // called with SurfaceFlinger::mStateLock as soon as the layer is entered // in the purgatory list void Layer::onRemoved() @@ -626,11 +634,10 @@ void Layer::dump(String8& result, char* buffer, size_t SIZE) const // --------------------------------------------------------------------------- Layer::ClientRef::ClientRef() - : mToken(-1) { + : mControlBlock(0), mToken(-1) { } Layer::ClientRef::~ClientRef() { - delete lcblk; } int32_t Layer::ClientRef::getToken() const { @@ -638,14 +645,25 @@ int32_t Layer::ClientRef::getToken() const { return mToken; } -status_t Layer::ClientRef::setToken(const sp& uc, - SharedBufferServer* sharedClient, int32_t token) { +sp Layer::ClientRef::getClient() const { Mutex::Autolock _l(mLock); - if (mToken >= 0) - return INVALID_OPERATION; + return mUserClient.promote(); +} + +status_t Layer::ClientRef::setToken(const sp& uc, + const sp& sharedClient, int32_t token) { + Mutex::Autolock _l(mLock); + + { // scope for strong mUserClient reference + sp userClient(mUserClient.promote()); + if (mUserClient != 0 && mControlBlock != 0) { + mControlBlock->setStatus(NO_INIT); + } + } + mUserClient = uc; mToken = token; - lcblk = sharedClient; + mControlBlock = sharedClient; return NO_ERROR; } @@ -657,12 +675,16 @@ sp Layer::ClientRef::getUserClientUnsafe() const { // it makes sure the UserClient (and its associated shared memory) // won't go away while we're accessing it. Layer::ClientRef::Access::Access(const ClientRef& ref) - : lcblk(0) + : mControlBlock(0) { Mutex::Autolock _l(ref.mLock); mUserClientStrongRef = ref.mUserClient.promote(); if (mUserClientStrongRef != 0) - lcblk = ref.lcblk; + mControlBlock = ref.mControlBlock; +} + +Layer::ClientRef::Access::~Access() +{ } // --------------------------------------------------------------------------- diff --git a/libs/surfaceflinger/Layer.h b/libs/surfaceflinger/Layer.h index d396ecfc3..dcb27a002 100644 --- a/libs/surfaceflinger/Layer.h +++ b/libs/surfaceflinger/Layer.h @@ -60,6 +60,7 @@ public: // associate a UserClient to this Layer status_t setToken(const sp& uc, SharedClient* sc, int32_t idx); int32_t getToken() const; + sp getClient() const; // Set this Layer's buffers size void setBufferSize(uint32_t w, uint32_t h); @@ -119,24 +120,26 @@ private: ClientRef& operator = (const ClientRef& rhs); mutable Mutex mLock; // binder thread, page-flip thread - SharedBufferServer* lcblk; + sp mControlBlock; wp mUserClient; int32_t mToken; public: ClientRef(); ~ClientRef(); int32_t getToken() const; + sp getClient() const; status_t setToken(const sp& uc, - SharedBufferServer* sharedClient, int32_t token); + const sp& sharedClient, int32_t token); sp getUserClientUnsafe() const; class Access { Access(const Access& rhs); Access& operator = (const Access& rhs); sp mUserClientStrongRef; - SharedBufferServer* lcblk; + sp mControlBlock; public: Access(const ClientRef& ref); - inline SharedBufferServer* get() const { return lcblk; } + ~Access(); + inline SharedBufferServer* get() const { return mControlBlock.get(); } }; friend class Access; }; diff --git a/libs/surfaceflinger/SurfaceFlinger.cpp b/libs/surfaceflinger/SurfaceFlinger.cpp index 49268588b..4dea62f41 100644 --- a/libs/surfaceflinger/SurfaceFlinger.cpp +++ b/libs/surfaceflinger/SurfaceFlinger.cpp @@ -1718,7 +1718,10 @@ void UserClient::detachLayer(const Layer* layer) { int32_t name = layer->getToken(); if (name >= 0) { - android_atomic_and(~(1LU<& sur) const sp layer(mFlinger->getLayer(sur)); if (layer == 0) return name; - // this layer already has a token, just return it - // FIXME: we should check that this token is for the same client + // if this layer already has a token, just return it name = layer->getToken(); - if (name >= 0) return name; + if ((name >= 0) && (layer->getClient() == this)) + return name; name = 0; do { int32_t mask = 1LU<setToken(const_cast(this), ctrlblk, name); + status_t err = layer->setToken( + const_cast(this), ctrlblk, name); + if (err != NO_ERROR) { + // free the name + android_atomic_and(~mask, &mBitmap); + name = err; + } break; } if (++name > 31) diff --git a/libs/surfaceflinger_client/SharedBufferStack.cpp b/libs/surfaceflinger_client/SharedBufferStack.cpp index 1dd864251..d67a589ac 100644 --- a/libs/surfaceflinger_client/SharedBufferStack.cpp +++ b/libs/surfaceflinger_client/SharedBufferStack.cpp @@ -494,6 +494,10 @@ SharedBufferServer::SharedBufferServer(SharedClient* sharedClient, } } +SharedBufferServer::~SharedBufferServer() +{ +} + ssize_t SharedBufferServer::retireAndLock() { RWLock::AutoRLock _l(mLock); diff --git a/libs/surfaceflinger_client/Surface.cpp b/libs/surfaceflinger_client/Surface.cpp index 6fe4c4ac6..8617d94a4 100644 --- a/libs/surfaceflinger_client/Surface.cpp +++ b/libs/surfaceflinger_client/Surface.cpp @@ -236,17 +236,15 @@ status_t SurfaceControl::validate() const status_t SurfaceControl::writeSurfaceToParcel( const sp& control, Parcel* parcel) { - uint32_t flags = 0; - uint32_t format = 0; + sp sur; uint32_t identity = 0; uint32_t width = 0; uint32_t height = 0; - sp client; - sp sur; + uint32_t format = 0; + uint32_t flags = 0; if (SurfaceControl::isValid(control)) { - identity = control->mIdentity; - client = control->mClient; sur = control->mSurface; + identity = control->mIdentity; width = control->mWidth; height = control->mHeight; format = control->mFormat; @@ -349,6 +347,33 @@ Surface::Surface(const Parcel& parcel, const sp& ref) init(); } +status_t Surface::writeToParcel( + const sp& surface, Parcel* parcel) +{ + sp sur; + uint32_t identity = 0; + uint32_t width = 0; + uint32_t height = 0; + uint32_t format = 0; + uint32_t flags = 0; + if (Surface::isValid(surface)) { + sur = surface->mSurface; + identity = surface->mIdentity; + width = surface->mWidth; + height = surface->mHeight; + format = surface->mFormat; + flags = surface->mFlags; + } + parcel->writeStrongBinder(sur!=0 ? sur->asBinder() : NULL); + parcel->writeInt32(identity); + parcel->writeInt32(width); + parcel->writeInt32(height); + parcel->writeInt32(format); + parcel->writeInt32(flags); + return NO_ERROR; + +} + sp Surface::readFromParcel( const Parcel& data, const sp& other) { @@ -385,11 +410,11 @@ void Surface::init() mBuffers.insertAt(0, 2); if (mSurface != 0 && mClient.initCheck() == NO_ERROR) { - mToken = mClient.getTokenForSurface(mSurface); - if (mToken >= 0) { + int32_t token = mClient.getTokenForSurface(mSurface); + if (token >= 0) { mSharedBufferClient = new SharedBufferClient( - mClient.getSharedClient(), mToken, 2, mIdentity); - mInitCheck = mClient.getSharedClient()->validate(mToken); + mClient.getSharedClient(), token, 2, mIdentity); + mInitCheck = mClient.getSharedClient()->validate(token); } } } @@ -421,7 +446,7 @@ status_t Surface::validate() const { // check that we initialized ourself properly if (mInitCheck != NO_ERROR) { - LOGE("invalid token (%d, identity=%u)", mToken, mIdentity); + LOGE("invalid token (identity=%u)", mIdentity); return mInitCheck; } @@ -437,17 +462,17 @@ status_t Surface::validate() const } if (mIdentity != identity) { - LOGE("[Surface] using an invalid surface id=%d, " + LOGE("[Surface] using an invalid surface, " "identity=%u should be %d", - mToken, mIdentity, identity); + mIdentity, identity); return NO_INIT; } // check the surface didn't become invalid status_t err = mSharedBufferClient->getStatus(); if (err != NO_ERROR) { - LOGE("surface (id=%d, identity=%u) is invalid, err=%d (%s)", - mToken, mIdentity, err, strerror(-err)); + LOGE("surface (identity=%u) is invalid, err=%d (%s)", + mIdentity, err, strerror(-err)); return err; } diff --git a/libs/ui/GraphicBuffer.cpp b/libs/ui/GraphicBuffer.cpp index 3ddde38f2..4b5f02517 100644 --- a/libs/ui/GraphicBuffer.cpp +++ b/libs/ui/GraphicBuffer.cpp @@ -111,6 +111,9 @@ status_t GraphicBuffer::reallocate(uint32_t w, uint32_t h, PixelFormat f, if (mOwner != ownData) return INVALID_OPERATION; + if (handle && w==width && h==height && f==format && reqUsage==usage) + return NO_ERROR; + if (handle) { GraphicBufferAllocator& allocator(GraphicBufferAllocator::get()); allocator.free(handle);