From: Mathias Agopian Date: Mon, 15 Oct 2012 23:51:41 +0000 (-0700) Subject: fix a corruption in blank/unblank X-Git-Tag: android-x86-4.4-r1~422^2~7^2~27 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=db9b41fd;p=android-x86%2Fframeworks-native.git fix a corruption in blank/unblank we were holding a reference (ie: pointer) to a sp 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 --- diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index e21e2bfc35..13b2c6be25 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -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 hw = getDefaultDisplayDevice(); + sp 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& hw(getDefaultDisplayDevice()); + const sp 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& disp(getDisplayDevice(display)); + const sp 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& hw) { void SurfaceFlinger::unblank(const sp& display) { class MessageScreenAcquired : public MessageBase { - SurfaceFlinger* mFlinger; - const sp& mHw; + SurfaceFlinger& mFlinger; + sp mDisplay; public: - MessageScreenAcquired(SurfaceFlinger* flinger, - const sp& hw) : mFlinger(flinger), mHw(hw) { } + MessageScreenAcquired(SurfaceFlinger& flinger, + const sp& disp) : mFlinger(flinger), mDisplay(disp) { } virtual bool handler() { - mFlinger->onScreenAcquired(mHw); + const sp hw(mFlinger.getDisplayDevice(mDisplay)); + if (hw == NULL) { + ALOGE("Attempt to unblank null display %p", mDisplay.get()); + } else if (hw->getDisplayType() >= DisplayDevice::NUM_DISPLAY_TYPES) { + ALOGW("Attempt to unblank virtual display"); + } else { + mFlinger.onScreenAcquired(hw); + } return true; } }; - const sp& hw = getDisplayDevice(display); - if (hw == NULL) { - ALOGE("Attempt to unblank null display %p", display.get()); - } else if (hw->getDisplayType() >= DisplayDevice::NUM_DISPLAY_TYPES) { - ALOGW("Attempt to unblank virtual display"); - } else { - sp msg = new MessageScreenAcquired(this, hw); - postMessageSync(msg); - } + sp msg = new MessageScreenAcquired(*this, display); + postMessageSync(msg); } void SurfaceFlinger::blank(const sp& display) { class MessageScreenReleased : public MessageBase { - SurfaceFlinger* mFlinger; - const sp& mHw; + SurfaceFlinger& mFlinger; + sp mDisplay; public: - MessageScreenReleased(SurfaceFlinger* flinger, - const sp& hw) : mFlinger(flinger), mHw(hw) { } + MessageScreenReleased(SurfaceFlinger& flinger, + const sp& disp) : mFlinger(flinger), mDisplay(disp) { } virtual bool handler() { - mFlinger->onScreenReleased(mHw); + const sp hw(mFlinger.getDisplayDevice(mDisplay)); + if (hw == NULL) { + ALOGE("Attempt to blank null display %p", mDisplay.get()); + } else if (hw->getDisplayType() >= DisplayDevice::NUM_DISPLAY_TYPES) { + ALOGW("Attempt to blank virtual display"); + } else { + mFlinger.onScreenReleased(hw); + } return true; } }; - const sp& hw = getDisplayDevice(display); - if (hw == NULL) { - ALOGE("Attempt to blank null display %p", display.get()); - } else if (hw->getDisplayType() >= DisplayDevice::NUM_DISPLAY_TYPES) { - ALOGW("Attempt to blank virtual display"); - } else { - sp msg = new MessageScreenReleased(this, hw); - postMessageSync(msg); - } + sp msg = new MessageScreenReleased(*this, display); + postMessageSync(msg); } // --------------------------------------------------------------------------- @@ -2359,6 +2359,7 @@ void SurfaceFlinger::dumpAllLocked( const Vector< sp >& SurfaceFlinger::getLayerSortedByZForHwcDisplay(int disp) { + // Note: mStateLock is held here return getDisplayDevice( getBuiltInDisplay(disp) )->getVisibleLayersSortedByZ(); } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index efe34afc2b..de97167812 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -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 getDisplayDevice(const wp& dpy) const { return mDisplays.valueFor(dpy); } - const sp& getDisplayDevice(const wp& dpy) { + + // NOTE: can only be called from the main thread or with mStateLock held + sp getDisplayDevice(const wp& 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, sp > mDisplays; // don't use a lock for these, we don't care