Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9ec430ad authored by Mathias Agopian's avatar Mathias Agopian
Browse files

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.
parent ef864634

include/private/ui/SharedBufferStack.h

+13 −7
Original line number Diff line number Diff line
@@ -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,7 +228,8 @@ 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);

libs/surfaceflinger/Layer.cpp

+2 −2
Original line number Diff line number Diff line
@@ -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;

libs/surfaceflinger/Layer.h

+1 −1
Original line number Diff line number Diff line
@@ -80,7 +80,7 @@ public: 
    inline PixelFormat pixelFormat() const { return mFormat; }



private:

    inline sp<GraphicBuffer> getFrontBuffer() {

    inline sp<GraphicBuffer> getFrontBufferLocked() {

        return mBuffers[mFrontBufferIndex];

    }

libs/surfaceflinger/LayerBlur.cpp

+2 −2
Original line number Diff line number Diff line
@@ -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,

libs/ui/SharedBufferStack.cpp

+5 −5
Original line number Diff line number Diff line
@@ -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;