ADHOC/replicant-frameworks_native
3
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

fix a corruption in blank/unblank

Browse Source 
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
This commit is contained in:
Mathias Agopian 2012-10-15 16:51:41 -07:00 committed by Android (Google) Code Review
parent 3365c56716
commit db9b41fd15
2 changed files with 39 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

63
services/surfaceflinger/SurfaceFlinger.cpp
View File

@ -507,7 +507,7 @@ status_t SurfaceFlinger::readyToRun()
// or when a texture is (asynchronously) destroyed, and for that // or when a texture is (asynchronously) destroyed, and for that
// we need a valid surface, so it's convenient to use the main display // we need a valid surface, so it's convenient to use the main display
// for that. // for that.
sp<const DisplayDevice> hw = getDefaultDisplayDevice(); sp<const DisplayDevice> hw(getDefaultDisplayDevice());
// initialize OpenGL ES // initialize OpenGL ES
DisplayDevice::makeCurrent(mEGLDisplay, hw, mEGLContext); DisplayDevice::makeCurrent(mEGLDisplay, hw, mEGLContext);
@ -1126,7 +1126,7 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags)
// Call makeCurrent() on the primary display so we can // Call makeCurrent() on the primary display so we can
// be sure that nothing associated with this display // be sure that nothing associated with this display
// is current. // is current.
const sp<const DisplayDevice>& hw(getDefaultDisplayDevice()); const sp<const DisplayDevice> hw(getDefaultDisplayDevice());
DisplayDevice::makeCurrent(mEGLDisplay, hw, mEGLContext); DisplayDevice::makeCurrent(mEGLDisplay, hw, mEGLContext);
mDisplays.removeItem(draw.keyAt(i)); mDisplays.removeItem(draw.keyAt(i));
getHwComposer().disconnectDisplay(draw[i].type); getHwComposer().disconnectDisplay(draw[i].type);
@ -1150,7 +1150,7 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags)
continue; continue;
} }
const sp<DisplayDevice>& disp(getDisplayDevice(display)); const sp<DisplayDevice> disp(getDisplayDevice(display));
if (disp != NULL) { if (disp != NULL) {
if (state.layerStack != draw[i].layerStack) { if (state.layerStack != draw[i].layerStack) {
disp->setLayerStack(state.layerStack); disp->setLayerStack(state.layerStack);
@ -2043,48 +2043,48 @@ void SurfaceFlinger::onScreenReleased(const sp<const DisplayDevice>& hw) {
void SurfaceFlinger::unblank(const sp<IBinder>& display) { void SurfaceFlinger::unblank(const sp<IBinder>& display) {
class MessageScreenAcquired : public MessageBase { class MessageScreenAcquired : public MessageBase {
SurfaceFlinger* mFlinger; SurfaceFlinger& mFlinger;
const sp<DisplayDevice>& mHw; sp<IBinder> mDisplay;
public: public:
MessageScreenAcquired(SurfaceFlinger* flinger, MessageScreenAcquired(SurfaceFlinger& flinger,
const sp<DisplayDevice>& hw) : mFlinger(flinger), mHw(hw) { } const sp<IBinder>& disp) : mFlinger(flinger), mDisplay(disp) { }
virtual bool handler() { virtual bool handler() {
mFlinger->onScreenAcquired(mHw); const sp<DisplayDevice> 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; return true;
} }
}; };
const sp<DisplayDevice>& hw = getDisplayDevice(display); sp<MessageBase> msg = new MessageScreenAcquired(*this, display);
if (hw == NULL) { postMessageSync(msg);
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<MessageBase> msg = new MessageScreenAcquired(this, hw);
postMessageSync(msg);
}
} }
void SurfaceFlinger::blank(const sp<IBinder>& display) { void SurfaceFlinger::blank(const sp<IBinder>& display) {
class MessageScreenReleased : public MessageBase { class MessageScreenReleased : public MessageBase {
SurfaceFlinger* mFlinger; SurfaceFlinger& mFlinger;
const sp<DisplayDevice>& mHw; sp<IBinder> mDisplay;
public: public:
MessageScreenReleased(SurfaceFlinger* flinger, MessageScreenReleased(SurfaceFlinger& flinger,
const sp<DisplayDevice>& hw) : mFlinger(flinger), mHw(hw) { } const sp<IBinder>& disp) : mFlinger(flinger), mDisplay(disp) { }
virtual bool handler() { virtual bool handler() {
mFlinger->onScreenReleased(mHw); const sp<DisplayDevice> 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; return true;
} }
}; };
const sp<DisplayDevice>& hw = getDisplayDevice(display); sp<MessageBase> msg = new MessageScreenReleased(*this, display);
if (hw == NULL) { postMessageSync(msg);
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<MessageBase> msg = new MessageScreenReleased(this, hw);
postMessageSync(msg);
}
} }
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
@ -2359,6 +2359,7 @@ void SurfaceFlinger::dumpAllLocked(
const Vector< sp<LayerBase> >& const Vector< sp<LayerBase> >&
SurfaceFlinger::getLayerSortedByZForHwcDisplay(int disp) { SurfaceFlinger::getLayerSortedByZForHwcDisplay(int disp) {
// Note: mStateLock is held here
return getDisplayDevice( getBuiltInDisplay(disp) )->getVisibleLayersSortedByZ(); return getDisplayDevice( getBuiltInDisplay(disp) )->getVisibleLayersSortedByZ();
} }

8
services/surfaceflinger/SurfaceFlinger.h
View File

@ -327,10 +327,13 @@ private:
// called when starting, or restarting after system_server death // called when starting, or restarting after system_server death
void initializeDisplays(); void initializeDisplays();
// NOTE: can only be called from the main thread or with mStateLock held
sp<const DisplayDevice> getDisplayDevice(const wp<IBinder>& dpy) const { sp<const DisplayDevice> getDisplayDevice(const wp<IBinder>& dpy) const {
return mDisplays.valueFor(dpy); 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); return mDisplays.valueFor(dpy);
} }
@ -425,6 +428,9 @@ private:
State mDrawingState; State mDrawingState;
bool mVisibleRegionsDirty; bool mVisibleRegionsDirty;
bool mHwWorkListDirty; 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; DefaultKeyedVector< wp<IBinder>, sp<DisplayDevice> > mDisplays;
// don't use a lock for these, we don't care // don't use a lock for these, we don't care