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

Commit db9b41fd authored by Mathias Agopian's avatar Mathias Agopian Committed by Android (Google) Code Review
Browse files

fix a corruption in blank/unblank 

we were holding a reference (ie: pointer) to a sp<DisplayDevice>
while processing the message. Meanwhile the object itself could
go away and we would end up accessing a dead object.

the root cause of the problem is that we are accessing mDisplays[]
in a few places outside of the main thread.

Bug: 7352770
Change-Id: I89e35dd85fb30e9a6383eca9a0bbc7028363876c
parent 3365c567

services/surfaceflinger/SurfaceFlinger.cpp

+32 −31
Original line number Diff line number Diff line
@@ -507,7 +507,7 @@ status_t SurfaceFlinger::readyToRun() 
    //  or when a texture is (asynchronously) destroyed, and for that

    //  we need a valid surface, so it's convenient to use the main display

    //  for that.

    sp<const DisplayDevice> hw = getDefaultDisplayDevice();

    sp<const DisplayDevice> hw(getDefaultDisplayDevice());



    //  initialize OpenGL ES

    DisplayDevice::makeCurrent(mEGLDisplay, hw, mEGLContext);
@@ -1126,7 +1126,7 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) 
                        // Call makeCurrent() on the primary display so we can

                        // be sure that nothing associated with this display

                        // is current.

                        const sp<const DisplayDevice>& hw(getDefaultDisplayDevice());

                        const sp<const DisplayDevice> hw(getDefaultDisplayDevice());

                        DisplayDevice::makeCurrent(mEGLDisplay, hw, mEGLContext);

                        mDisplays.removeItem(draw.keyAt(i));

                        getHwComposer().disconnectDisplay(draw[i].type);
@@ -1150,7 +1150,7 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) 
                        continue;

                    }



                    const sp<DisplayDevice>& disp(getDisplayDevice(display));

                    const sp<DisplayDevice> disp(getDisplayDevice(display));

                    if (disp != NULL) {

                        if (state.layerStack != draw[i].layerStack) {

                            disp->setLayerStack(state.layerStack);
@@ -2043,48 +2043,48 @@ void SurfaceFlinger::onScreenReleased(const sp<const DisplayDevice>& hw) { 


void SurfaceFlinger::unblank(const sp<IBinder>& display) {

    class MessageScreenAcquired : public MessageBase {

        SurfaceFlinger* mFlinger;

        const sp<DisplayDevice>& mHw;

        SurfaceFlinger& mFlinger;

        sp<IBinder> mDisplay;

    public:

        MessageScreenAcquired(SurfaceFlinger* flinger,

                const sp<DisplayDevice>& hw) : mFlinger(flinger), mHw(hw) { }

        MessageScreenAcquired(SurfaceFlinger& flinger,

                const sp<IBinder>& disp) : mFlinger(flinger), mDisplay(disp) { }

        virtual bool handler() {

            mFlinger->onScreenAcquired(mHw);

            return true;

        }

    };

    const sp<DisplayDevice>& hw = getDisplayDevice(display);

            const sp<DisplayDevice> hw(mFlinger.getDisplayDevice(mDisplay));

            if (hw == NULL) {

        ALOGE("Attempt to unblank null display %p", display.get());

                ALOGE("Attempt to unblank null display %p", mDisplay.get());

            } else if (hw->getDisplayType() >= DisplayDevice::NUM_DISPLAY_TYPES) {

                ALOGW("Attempt to unblank virtual display");

            } else {

        sp<MessageBase> msg = new MessageScreenAcquired(this, hw);

        postMessageSync(msg);

                mFlinger.onScreenAcquired(hw);

            }

            return true;

        }

    };

    sp<MessageBase> msg = new MessageScreenAcquired(*this, display);

    postMessageSync(msg);

}



void SurfaceFlinger::blank(const sp<IBinder>& display) {

    class MessageScreenReleased : public MessageBase {

        SurfaceFlinger* mFlinger;

        const sp<DisplayDevice>& mHw;

        SurfaceFlinger& mFlinger;

        sp<IBinder> mDisplay;

    public:

        MessageScreenReleased(SurfaceFlinger* flinger,

                const sp<DisplayDevice>& hw) : mFlinger(flinger), mHw(hw) { }

        MessageScreenReleased(SurfaceFlinger& flinger,

                const sp<IBinder>& disp) : mFlinger(flinger), mDisplay(disp) { }

        virtual bool handler() {

            mFlinger->onScreenReleased(mHw);

            return true;

        }

    };

    const sp<DisplayDevice>& hw = getDisplayDevice(display);

            const sp<DisplayDevice> hw(mFlinger.getDisplayDevice(mDisplay));

            if (hw == NULL) {

        ALOGE("Attempt to blank null display %p", display.get());

                ALOGE("Attempt to blank null display %p", mDisplay.get());

            } else if (hw->getDisplayType() >= DisplayDevice::NUM_DISPLAY_TYPES) {

                ALOGW("Attempt to blank virtual display");

            } else {

        sp<MessageBase> msg = new MessageScreenReleased(this, hw);

        postMessageSync(msg);

                mFlinger.onScreenReleased(hw);

            }

            return true;

        }

    };

    sp<MessageBase> msg = new MessageScreenReleased(*this, display);

    postMessageSync(msg);

}



// ---------------------------------------------------------------------------
@@ -2359,6 +2359,7 @@ void SurfaceFlinger::dumpAllLocked( 


const Vector< sp<LayerBase> >&

SurfaceFlinger::getLayerSortedByZForHwcDisplay(int disp) {

    // Note: mStateLock is held here

    return getDisplayDevice( getBuiltInDisplay(disp) )->getVisibleLayersSortedByZ();

}

services/surfaceflinger/SurfaceFlinger.h

+7 −1
Original line number Diff line number Diff line
@@ -327,10 +327,13 @@ private: 
    // called when starting, or restarting after system_server death

    void initializeDisplays();



    // NOTE: can only be called from the main thread or with mStateLock held

    sp<const DisplayDevice> getDisplayDevice(const wp<IBinder>& dpy) const {

        return mDisplays.valueFor(dpy);

    }

    const sp<DisplayDevice>& getDisplayDevice(const wp<IBinder>& dpy) {



    // NOTE: can only be called from the main thread or with mStateLock held

    sp<DisplayDevice> getDisplayDevice(const wp<IBinder>& dpy) {

        return mDisplays.valueFor(dpy);

    }
@@ -425,6 +428,9 @@ private: 
    State mDrawingState;

    bool mVisibleRegionsDirty;

    bool mHwWorkListDirty;



    // this may only be written from the main thread with mStateLock held

    // it may be read from other threads with mStateLock held

    DefaultKeyedVector< wp<IBinder>, sp<DisplayDevice> > mDisplays;



    // don't use a lock for these, we don't care