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

Commit 359140c1 authored by Mathias Agopian's avatar Mathias Agopian
Browse files

free gralloc buffers as soon as possible (when a surface is not visible any...

free gralloc buffers as soon as possible (when a surface is not visible any longer), client who have the buffers still mapped won't crash, btu may see garbage data
parent dfe983bd
Loading
Loading
Loading
Loading
+2 −1
Original line number Diff line number Diff line
@@ -76,7 +76,9 @@ void Layer::destroy()
            eglDestroyImageKHR(dpy, mTextures[i].image);
            mTextures[i].image = EGL_NO_IMAGE_KHR;
        }
        mBuffers[i].free();
    }
    mSurface.clear();
}

void Layer::initStates(uint32_t w, uint32_t h, uint32_t flags)
@@ -95,7 +97,6 @@ sp<LayerBaseClient::Surface> Layer::createSurface() const
status_t Layer::ditch()
{
    // the layer is not on screen anymore. free as much resources as possible
    mSurface.clear();
    destroy();
    return NO_ERROR;
}
+10 −4
Original line number Diff line number Diff line
@@ -130,13 +130,10 @@ status_t Buffer::lock(GGLSurface* sur, uint32_t usage)
    return res;
}



// ===========================================================================
// LayerBitmap
// ===========================================================================


LayerBitmap::LayerBitmap()
    : mInfo(0), mWidth(0), mHeight(0)
{
@@ -184,7 +181,7 @@ sp<Buffer> LayerBitmap::allocate()
    sp<Buffer> buffer(mBuffer);
    const uint32_t w = mWidth; 
    const uint32_t h = mHeight;
    if (w != buffer->getWidth() || h != buffer->getHeight()) {
    if (buffer!=0 && (w != buffer->getWidth() || h != buffer->getHeight())) {
        surface_info_t* info = mInfo;
        buffer = new Buffer(w, h, mFormat, mFlags);
        status_t err = buffer->initCheck();
@@ -200,6 +197,15 @@ sp<Buffer> LayerBitmap::allocate()
    return buffer;
}

status_t LayerBitmap::free()
{
    mBuffer.clear();
    mWidth = 0;
    mHeight = 0;
    return NO_ERROR;
}


// ---------------------------------------------------------------------------

}; // namespace android
+2 −1
Original line number Diff line number Diff line
@@ -110,6 +110,7 @@ public:
    status_t setSize(uint32_t w, uint32_t h);

    sp<Buffer> allocate();
    status_t free();
    
    sp<const Buffer>  getBuffer() const { return mBuffer; }
    sp<Buffer>        getBuffer()       { return mBuffer; }
+14 −30
Original line number Diff line number Diff line
@@ -1095,12 +1095,9 @@ status_t SurfaceFlinger::removeLayer_l(const sp<LayerBase>& layerBase)

status_t SurfaceFlinger::purgatorizeLayer_l(const sp<LayerBase>& layerBase)
{
    // First add the layer to the purgatory list, which makes sure it won't 
    // go away, then remove it from the main list (through a transaction).
    // remove the layer from the main list (through a transaction).
    ssize_t err = removeLayer_l(layerBase);
    if (err >= 0) {
        mLayerPurgatory.add(layerBase);
    }

    // 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(),
@@ -1336,7 +1333,7 @@ status_t SurfaceFlinger::removeSurface(SurfaceID index)

status_t SurfaceFlinger::destroySurface(const sp<LayerBaseClient>& layer)
{
    /* called by ~ISurface() when all references are gone */
    // called by ~ISurface() when all references are gone
    
    class MessageDestroySurface : public MessageBase {
        SurfaceFlinger* flinger;
@@ -1349,33 +1346,21 @@ status_t SurfaceFlinger::destroySurface(const sp<LayerBaseClient>& layer)
            sp<LayerBaseClient> l(layer);
            layer.clear(); // clear it outside of the lock;
            Mutex::Autolock _l(flinger->mStateLock);
            // remove the layer from the current list -- chances are that it's 
            // not in the list anyway, because it should have been removed 
            // already upon request of the client (eg: window manager). 
            // However, a buggy client could have not done that.
            // Since we know we don't have any more clients, we don't need
            // to use the purgatory.
            /*
             * remove the layer from the current list -- chances are that it's 
             * not in the list anyway, because it should have been removed 
             * already upon request of the client (eg: window manager). 
             * However, a buggy client could have not done that.
             * Since we know we don't have any more clients, we don't need
             * to use the purgatory.
             */
            status_t err = flinger->removeLayer_l(l);
            if (err == NAME_NOT_FOUND) {
                // The surface wasn't in the current list, which means it was
                // removed already, which means it is in the purgatory, 
                // and need to be removed from there.
                // This needs to happen from the main thread since its dtor
                // must run from there (b/c of OpenGL ES). Additionally, we
                // can't really acquire our internal lock from 
                // destroySurface() -- see postMessage() below.
                ssize_t idx = flinger->mLayerPurgatory.remove(l);
                LOGE_IF(idx < 0,
                        "layer=%p is not in the purgatory list", l.get());
            }
            LOGE_IF(err<0 && err != NAME_NOT_FOUND,
                    "error removing layer=%p (%s)", l.get(), strerror(-err));
            return true;
        }
    };

    // It's better to not acquire our internal lock here, because it's hard
    // to predict that it's not going to be already taken when ~Surface()
    // is called.

    mEventQueue.postMessage( new MessageDestroySurface(this, layer) );
    return NO_ERROR;
}
@@ -1537,8 +1522,7 @@ 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, client count: %d\n",
                mLayerPurgatory.size(), mClientsMap.size());
        snprintf(buffer, SIZE, "  client count: %d\n", mClientsMap.size());
        result.append(buffer);
        const BufferAllocator& alloc(BufferAllocator::get());
        alloc.dump(result);
+0 −2
Original line number Diff line number Diff line
@@ -307,8 +307,6 @@ private:
    volatile    int32_t                 mTransactionFlags;
    volatile    int32_t                 mTransactionCount;
                Condition               mTransactionCV;
                SortedVector< sp<LayerBase> > mLayerPurgatory;

                
                // protected by mStateLock (but we could use another lock)
                Tokenizer                               mTokens;