diff options
author | Mathias Agopian <mathias@google.com> | 2009-10-02 18:12:30 -0700 |
---|---|---|
committer | Mathias Agopian <mathias@google.com> | 2009-10-02 18:12:30 -0700 |
commit | 0c4cec7e4df87181486d280c98fba9c0f4774c37 (patch) | |
tree | 4d3aa4fe0101104a1816af9208d367d588bb4f4d /libs | |
parent | 7b16834adc1003f492cd2be4b1bcc3fb73a78c23 (diff) | |
download | frameworks_base-0c4cec7e4df87181486d280c98fba9c0f4774c37.zip frameworks_base-0c4cec7e4df87181486d280c98fba9c0f4774c37.tar.gz frameworks_base-0c4cec7e4df87181486d280c98fba9c0f4774c37.tar.bz2 |
Attempt to fix [2152536] ANR in browser
The ANR is caused by SurfaceFlinger waiting for buffers of a removed surface to become availlable.
When it is removed from the current list, a Surface is marked as NO_INIT, which causes SF to return
immediately in the above case. For some reason, the surface here wasn't marked as NO_INIT.
This change makes the code more robust by always (irregadless or errors) setting the NO_INIT status
in all code paths where a surface is removed from the list.
Additionaly added more information in the logs, should this happen again.
Diffstat (limited to 'libs')
-rw-r--r-- | libs/surfaceflinger/Buffer.cpp | 5 | ||||
-rw-r--r-- | libs/surfaceflinger/Buffer.h | 11 | ||||
-rw-r--r-- | libs/surfaceflinger/BufferAllocator.cpp | 2 | ||||
-rw-r--r-- | libs/surfaceflinger/Layer.cpp | 8 | ||||
-rw-r--r-- | libs/surfaceflinger/Layer.h | 2 | ||||
-rw-r--r-- | libs/surfaceflinger/LayerBase.cpp | 9 | ||||
-rw-r--r-- | libs/surfaceflinger/LayerBase.h | 9 | ||||
-rw-r--r-- | libs/surfaceflinger/SurfaceFlinger.cpp | 3 | ||||
-rw-r--r-- | libs/ui/SharedBufferStack.cpp | 13 | ||||
-rw-r--r-- | libs/ui/Surface.cpp | 8 |
10 files changed, 31 insertions, 39 deletions
diff --git a/libs/surfaceflinger/Buffer.cpp b/libs/surfaceflinger/Buffer.cpp index 65650aa..6190cd8 100644 --- a/libs/surfaceflinger/Buffer.cpp +++ b/libs/surfaceflinger/Buffer.cpp @@ -20,17 +20,12 @@ #include <utils/Errors.h> #include <utils/Log.h> -#include <binder/MemoryBase.h> -#include <binder/IMemory.h> #include <ui/PixelFormat.h> -#include <ui/Surface.h> #include <pixelflinger/pixelflinger.h> #include "Buffer.h" #include "BufferAllocator.h" -#include "SurfaceFlinger.h" - namespace android { diff --git a/libs/surfaceflinger/Buffer.h b/libs/surfaceflinger/Buffer.h index 79f4eeb..203da3b 100644 --- a/libs/surfaceflinger/Buffer.h +++ b/libs/surfaceflinger/Buffer.h @@ -20,20 +20,11 @@ #include <stdint.h> #include <sys/types.h> -#include <hardware/gralloc.h> - -#include <utils/Atomic.h> - #include <ui/PixelFormat.h> #include <ui/Rect.h> -#include <ui/Surface.h> - #include <pixelflinger/pixelflinger.h> - -#include <private/ui/SharedBufferStack.h> #include <private/ui/SurfaceBuffer.h> -class copybit_image_t; struct android_native_buffer_t; namespace android { @@ -42,8 +33,6 @@ namespace android { // Buffer // =========================================================================== -class NativeBuffer; - class Buffer : public SurfaceBuffer { public: diff --git a/libs/surfaceflinger/BufferAllocator.cpp b/libs/surfaceflinger/BufferAllocator.cpp index caf9bec..3e37bc3 100644 --- a/libs/surfaceflinger/BufferAllocator.cpp +++ b/libs/surfaceflinger/BufferAllocator.cpp @@ -15,8 +15,6 @@ ** limitations under the License. */ -#include <sys/mman.h> -#include <cutils/ashmem.h> #include <cutils/log.h> #include <utils/Singleton.h> diff --git a/libs/surfaceflinger/Layer.cpp b/libs/surfaceflinger/Layer.cpp index 07222ec..13201db 100644 --- a/libs/surfaceflinger/Layer.cpp +++ b/libs/surfaceflinger/Layer.cpp @@ -65,14 +65,6 @@ Layer::~Layer() // the actual buffers will be destroyed here } -// called with SurfaceFlinger::mStateLock as soon as the layer is entered -// in the purgatory list -void Layer::onRemoved() -{ - // wake up the condition - lcblk->setStatus(NO_INIT); -} - void Layer::destroy() { for (size_t i=0 ; i<NUM_BUFFERS ; i++) { diff --git a/libs/surfaceflinger/Layer.h b/libs/surfaceflinger/Layer.h index 6c7b27b..f111840 100644 --- a/libs/surfaceflinger/Layer.h +++ b/libs/surfaceflinger/Layer.h @@ -85,8 +85,6 @@ private: return mBuffers[mFrontBufferIndex]; } - virtual void onRemoved(); - void reloadTexture(const Region& dirty); sp<SurfaceBuffer> requestBuffer(int index, int usage); diff --git a/libs/surfaceflinger/LayerBase.cpp b/libs/surfaceflinger/LayerBase.cpp index d83c842..83814cc 100644 --- a/libs/surfaceflinger/LayerBase.cpp +++ b/libs/surfaceflinger/LayerBase.cpp @@ -690,6 +690,14 @@ sp<LayerBaseClient::Surface> LayerBaseClient::createSurface() const const_cast<LayerBaseClient *>(this)); } +// called with SurfaceFlinger::mStateLock as soon as the layer is entered +// in the purgatory list +void LayerBaseClient::onRemoved() +{ + // wake up the condition + lcblk->setStatus(NO_INIT); +} + // --------------------------------------------------------------------------- LayerBaseClient::Surface::Surface( @@ -700,7 +708,6 @@ LayerBaseClient::Surface::Surface( { } - LayerBaseClient::Surface::~Surface() { /* diff --git a/libs/surfaceflinger/LayerBase.h b/libs/surfaceflinger/LayerBase.h index 16ee542..0dfa4fe 100644 --- a/libs/surfaceflinger/LayerBase.h +++ b/libs/surfaceflinger/LayerBase.h @@ -205,10 +205,13 @@ public: */ virtual bool isSecure() const { return false; } - /** signal this layer that it's not needed any longer. called from the - * main thread */ + /** Called from the main thread, when the surface is removed from the + * draw list */ virtual status_t ditch() { return NO_ERROR; } + /** called with the state lock when the surface is removed from the + * current list */ + virtual void onRemoved() { }; enum { // flags for doTransaction() @@ -318,7 +321,7 @@ public: sp<Surface> getSurface(); virtual sp<Surface> createSurface() const; - virtual void onRemoved() { } + virtual void onRemoved(); class Surface : public BnSurface { diff --git a/libs/surfaceflinger/SurfaceFlinger.cpp b/libs/surfaceflinger/SurfaceFlinger.cpp index eb0983a..f2b918f 100644 --- a/libs/surfaceflinger/SurfaceFlinger.cpp +++ b/libs/surfaceflinger/SurfaceFlinger.cpp @@ -1073,6 +1073,8 @@ status_t SurfaceFlinger::purgatorizeLayer_l(const sp<LayerBase>& layerBase) // remove the layer from the main list (through a transaction). ssize_t err = removeLayer_l(layerBase); + layerBase->onRemoved(); + // it's possible that we don't find a layer, because it might // have been destroyed already -- this is not technically an error // from the user because there is a race between BClient::destroySurface(), @@ -1321,7 +1323,6 @@ status_t SurfaceFlinger::removeSurface(SurfaceID index) if (layer != 0) { err = purgatorizeLayer_l(layer); if (err == NO_ERROR) { - layer->onRemoved(); setTransactionFlags(eTransactionNeeded); } } diff --git a/libs/ui/SharedBufferStack.cpp b/libs/ui/SharedBufferStack.cpp index 9ad4349..47c596c 100644 --- a/libs/ui/SharedBufferStack.cpp +++ b/libs/ui/SharedBufferStack.cpp @@ -114,6 +114,12 @@ uint32_t SharedBufferBase::getIdentity() return stack.identity; } +status_t SharedBufferBase::getStatus() const +{ + SharedBufferStack& stack( *mSharedStack ); + return stack.status; +} + size_t SharedBufferBase::getFrontBuffer() const { SharedBufferStack& stack( *mSharedStack ); @@ -135,7 +141,6 @@ String8 SharedBufferBase::dump(char const* prefix) const return result; } - // ============================================================================ // conditions and updates // ============================================================================ @@ -375,8 +380,10 @@ status_t SharedBufferServer::unlock(int buffer) void SharedBufferServer::setStatus(status_t status) { - StatusUpdate update(this, status); - updateCondition( update ); + if (status < NO_ERROR) { + StatusUpdate update(this, status); + updateCondition( update ); + } } status_t SharedBufferServer::reallocate() diff --git a/libs/ui/Surface.cpp b/libs/ui/Surface.cpp index 64522fb..285edb4 100644 --- a/libs/ui/Surface.cpp +++ b/libs/ui/Surface.cpp @@ -733,9 +733,11 @@ status_t Surface::getBufferLocked(int index, int usage) index, usage); if (buffer != 0) { // this should never happen by construction LOGE_IF(buffer->handle == NULL, - "requestBuffer(%d, %08x) returned a buffer with a null handle", - index, usage); - if (buffer->handle != NULL) { + "Surface (identity=%d) requestBuffer(%d, %08x) returned" + "a buffer with a null handle", mIdentity, index, usage); + err = mSharedBufferClient->getStatus(); + LOGE_IF(err, "Surface (identity=%d) state = %d", mIdentity, err); + if (!err && buffer->handle != NULL) { err = getBufferMapper().registerBuffer(buffer->handle); LOGW_IF(err, "registerBuffer(...) failed %d (%s)", err, strerror(-err)); |