From 2f4b68d21c1a58cbcb1e6929fb241e425a8f7b5d Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Fri, 2 Dec 2011 10:00:00 -0800 Subject: [PATCH] SurfaceFlinger: fix layer removal race condition Layer::lockPageFlip() and layer::onRemove() could be called on different threads and race such that lockPageFlip() successfully called mSurfaceTexture->updateTexImage() but then gets NULL back from mSurfaceTexture->getCurrentBuffer(), leading to a crash. This change moves Layer::onRemove() calls to SurfaceFlinger::commitTransaction() so they happen after the Layer is done being drawn from and only happen on the main surfaceflinger thread. Change-Id: I4b550caadff4cc1878d7c3bca6129193fb0c713e --- services/surfaceflinger/SurfaceFlinger.cpp | 10 +++++++++- services/surfaceflinger/SurfaceFlinger.h | 1 + 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index f38e94814..24bd2a63e 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -710,6 +710,14 @@ void SurfaceFlinger::computeVisibleRegions( void SurfaceFlinger::commitTransaction() { + if (!mLayersPendingRemoval.isEmpty()) { + // Notify removed layers now that they can't be drawn from + for (size_t i = 0; i < mLayersPendingRemoval.size(); i++) { + mLayersPendingRemoval[i]->onRemoved(); + } + mLayersPendingRemoval.clear(); + } + mDrawingState = mCurrentState; mTransationPending = false; mTransactionCV.broadcast(); @@ -1162,7 +1170,7 @@ status_t SurfaceFlinger::purgatorizeLayer_l(const sp& layerBase) mLayerPurgatory.add(layerBase); } - layerBase->onRemoved(); + mLayersPendingRemoval.push(layerBase); // it's possible that we don't find a layer, because it might // have been destroyed already -- this is not technically an error diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 17028dbb6..17b80a6ed 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -345,6 +345,7 @@ private: Condition mTransactionCV; SortedVector< sp > mLayerPurgatory; bool mTransationPending; + Vector< sp > mLayersPendingRemoval; // protected by mStateLock (but we could use another lock) GraphicPlane mGraphicPlanes[1];