summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Mathias Agopian <mathias@google.com> 2009-10-06 19:00:57 -0700
committer Mathias Agopian <mathias@google.com> 2009-10-06 19:00:57 -0700
commit9ec430adaea1cb88eaa1e78c7f759cd42ab6cf7a (patch)
tree1174b71aeaf88287d38538f8b7baa74532f6f638
parentef8646344a7168643b50a51ebffa2e41e9717608 (diff)
fix [2152536] ANR in browser
A window is created and the browser is about to render into it the very first time, at that point it does an IPC to SF to request a new buffer. Meanwhile, the window manager removes that window from the list and the shared memory block it uses is marked as invalid. However, at that point, another window is created and is given the same index (that just go freed), but a different identity and resets the "invalid" bit in the shared block. When we go back to the buffer allocation code, we're stuck because the surface we're allocating for is gone and we don't detect it's invalid because the invalid bit has been reset. It is not sufficient to check for the invalid bit, I should also check that identities match.
-rw-r--r--include/private/ui/SharedBufferStack.h20
-rw-r--r--libs/surfaceflinger/Layer.cpp4
-rw-r--r--libs/surfaceflinger/Layer.h2
-rw-r--r--libs/surfaceflinger/LayerBlur.cpp4
-rw-r--r--libs/ui/SharedBufferStack.cpp10
-rw-r--r--libs/ui/Surface.cpp4
6 files changed, 25 insertions, 19 deletions
diff --git a/include/private/ui/SharedBufferStack.h b/include/private/ui/SharedBufferStack.h
index 59cf31c89c..f6824d9950 100644
--- a/include/private/ui/SharedBufferStack.h
+++ b/include/private/ui/SharedBufferStack.h
@@ -139,7 +139,8 @@ private:
class SharedBufferBase
{
public:
- SharedBufferBase(SharedClient* sharedClient, int surface, int num);
+ SharedBufferBase(SharedClient* sharedClient, int surface, int num,
+ int32_t identity);
~SharedBufferBase();
uint32_t getIdentity();
status_t getStatus() const;
@@ -150,6 +151,7 @@ protected:
SharedClient* const mSharedClient;
SharedBufferStack* const mSharedStack;
const int mNumBuffers;
+ const int mIdentity;
friend struct Update;
friend struct QueueUpdate;
@@ -180,7 +182,10 @@ status_t SharedBufferBase::waitForCondition(T condition)
SharedClient& client( *mSharedClient );
const nsecs_t TIMEOUT = s2ns(1);
Mutex::Autolock _l(client.lock);
- while ((condition()==false) && (stack.status == NO_ERROR)) {
+ while ((condition()==false) &&
+ (stack.identity == mIdentity) &&
+ (stack.status == NO_ERROR))
+ {
status_t err = client.cv.waitRelative(client.lock, TIMEOUT);
// handle errors and timeouts
@@ -190,13 +195,13 @@ status_t SharedBufferBase::waitForCondition(T condition)
LOGE("waitForCondition(%s) timed out (identity=%d), "
"but condition is true! We recovered but it "
"shouldn't happen." , T::name(),
- mSharedStack->identity);
+ stack.identity);
break;
} else {
LOGW("waitForCondition(%s) timed out "
"(identity=%d, status=%d). "
"CPU may be pegged. trying again.", T::name(),
- mSharedStack->identity, mSharedStack->status);
+ stack.identity, stack.status);
}
} else {
LOGE("waitForCondition(%s) error (%s) ",
@@ -205,7 +210,7 @@ status_t SharedBufferBase::waitForCondition(T condition)
}
}
}
- return stack.status;
+ return (stack.identity != mIdentity) ? status_t(BAD_INDEX) : stack.status;
}
@@ -223,8 +228,9 @@ status_t SharedBufferBase::updateCondition(T update) {
class SharedBufferClient : public SharedBufferBase
{
public:
- SharedBufferClient(SharedClient* sharedClient, int surface, int num);
-
+ SharedBufferClient(SharedClient* sharedClient, int surface, int num,
+ int32_t identity);
+
ssize_t dequeue();
status_t undoDequeue(int buf);
diff --git a/libs/surfaceflinger/Layer.cpp b/libs/surfaceflinger/Layer.cpp
index eb0614fe1b..0258cee0db 100644
--- a/libs/surfaceflinger/Layer.cpp
+++ b/libs/surfaceflinger/Layer.cpp
@@ -133,7 +133,7 @@ status_t Layer::setBuffers( uint32_t w, uint32_t h,
void Layer::reloadTexture(const Region& dirty)
{
Mutex::Autolock _l(mLock);
- sp<GraphicBuffer> buffer(getFrontBuffer());
+ sp<GraphicBuffer> buffer(getFrontBufferLocked());
if (LIKELY((mFlags & DisplayHardware::DIRECT_TEXTURE) &&
(buffer->usage & GRALLOC_USAGE_HW_TEXTURE))) {
int index = mFrontBufferIndex;
@@ -194,7 +194,7 @@ void Layer::reloadTexture(const Region& dirty)
}
}
} else {
- for (int i=0 ; i<NUM_BUFFERS ; i++)
+ for (size_t i=0 ; i<NUM_BUFFERS ; i++)
mTextures[i].image = EGL_NO_IMAGE_KHR;
GGLSurface t;
diff --git a/libs/surfaceflinger/Layer.h b/libs/surfaceflinger/Layer.h
index 6f5924182d..702c51a55c 100644
--- a/libs/surfaceflinger/Layer.h
+++ b/libs/surfaceflinger/Layer.h
@@ -80,7 +80,7 @@ public:
inline PixelFormat pixelFormat() const { return mFormat; }
private:
- inline sp<GraphicBuffer> getFrontBuffer() {
+ inline sp<GraphicBuffer> getFrontBufferLocked() {
return mBuffers[mFrontBufferIndex];
}
diff --git a/libs/surfaceflinger/LayerBlur.cpp b/libs/surfaceflinger/LayerBlur.cpp
index 0ef663fe73..744f2e940d 100644
--- a/libs/surfaceflinger/LayerBlur.cpp
+++ b/libs/surfaceflinger/LayerBlur.cpp
@@ -189,8 +189,8 @@ void LayerBlur::onDraw(const Region& clip) const
} else {
GLuint tw = 1 << (31 - clz(w));
GLuint th = 1 << (31 - clz(h));
- if (tw < w) tw <<= 1;
- if (th < h) th <<= 1;
+ if (tw < GLuint(w)) tw <<= 1;
+ if (th < GLuint(h)) th <<= 1;
glTexImage2D(GL_TEXTURE_2D, 0, mReadFormat, tw, th, 0,
mReadFormat, mReadType, NULL);
glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, w, h,
diff --git a/libs/ui/SharedBufferStack.cpp b/libs/ui/SharedBufferStack.cpp
index 47c596c5c2..b460757414 100644
--- a/libs/ui/SharedBufferStack.cpp
+++ b/libs/ui/SharedBufferStack.cpp
@@ -97,10 +97,10 @@ Region SharedBufferStack::getDirtyRegion(int buffer) const
// ----------------------------------------------------------------------------
SharedBufferBase::SharedBufferBase(SharedClient* sharedClient,
- int surface, int num)
+ int surface, int num, int32_t identity)
: mSharedClient(sharedClient),
mSharedStack(sharedClient->surfaces + surface),
- mNumBuffers(num)
+ mNumBuffers(num), mIdentity(identity)
{
}
@@ -248,8 +248,8 @@ ssize_t SharedBufferServer::StatusUpdate::operator()() {
// ============================================================================
SharedBufferClient::SharedBufferClient(SharedClient* sharedClient,
- int surface, int num)
- : SharedBufferBase(sharedClient, surface, num), tail(0)
+ int surface, int num, int32_t identity)
+ : SharedBufferBase(sharedClient, surface, num, identity), tail(0)
{
tail = computeTail();
}
@@ -353,7 +353,7 @@ status_t SharedBufferClient::setDirtyRegion(int buffer, const Region& reg)
SharedBufferServer::SharedBufferServer(SharedClient* sharedClient,
int surface, int num, int32_t identity)
- : SharedBufferBase(sharedClient, surface, num)
+ : SharedBufferBase(sharedClient, surface, num, identity)
{
mSharedStack->init(identity);
mSharedStack->head = num-1;
diff --git a/libs/ui/Surface.cpp b/libs/ui/Surface.cpp
index 7822533ec9..2d83a8c310 100644
--- a/libs/ui/Surface.cpp
+++ b/libs/ui/Surface.cpp
@@ -314,7 +314,7 @@ Surface::Surface(const sp<SurfaceControl>& surface)
mWidth(surface->mWidth), mHeight(surface->mHeight)
{
mSharedBufferClient = new SharedBufferClient(
- mClient->mControl, mToken, 2);
+ mClient->mControl, mToken, 2, mIdentity);
init();
}
@@ -336,7 +336,7 @@ Surface::Surface(const Parcel& parcel)
mClient = SurfaceComposerClient::clientForConnection(clientBinder);
mSharedBufferClient = new SharedBufferClient(
- mClient->mControl, mToken, 2);
+ mClient->mControl, mToken, 2, mIdentity);
}
init();