Merge "SurfaceFlinger: add more thread-safety annotations."

5 files changed, 30 insertions, 12 deletions

diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index c2b0a112d6..905fe40600 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -72,6 +72,7 @@
 #include "LayerProtoHelper.h"
 #include "LayerRejecter.h"
 #include "MonitoredProducer.h"
+#include "MutexUtils.h"
 #include "SurfaceFlinger.h"
 #include "TimeStats/TimeStats.h"
 #include "TunnelModeEnabledReporter.h"
@@ -254,10 +255,12 @@ void Layer::onRemovedFromCurrentState() {
     auto layersInTree = getRootLayer()->getLayersInTree(LayerVector::StateSet::Current);
     std::sort(layersInTree.begin(), layersInTree.end());
 
-    traverse(LayerVector::StateSet::Current, [&](Layer* layer) {
-        layer->removeFromCurrentState();
-        layer->removeRelativeZ(layersInTree);
-    });
+    REQUIRE_MUTEX(mFlinger->mStateLock);
+    traverse(LayerVector::StateSet::Current,
+             [&](Layer* layer) REQUIRES(layer->mFlinger->mStateLock) {
+                 layer->removeFromCurrentState();
+                 layer->removeRelativeZ(layersInTree);
+             });
 }
 
 void Layer::addToCurrentState() {
@@ -936,10 +939,12 @@ bool Layer::setBackgroundColor(const half3& color, float alpha, ui::Dataspace da
         mFlinger->mLayersAdded = true;
         // set up SF to handle added color layer
         if (isRemovedFromCurrentState()) {
+            MUTEX_ALIAS(mFlinger->mStateLock, mDrawingState.bgColorLayer->mFlinger->mStateLock);
             mDrawingState.bgColorLayer->onRemovedFromCurrentState();
         }
         mFlinger->setTransactionFlags(eTransactionNeeded);
     } else if (mDrawingState.bgColorLayer && alpha == 0) {
+        MUTEX_ALIAS(mFlinger->mStateLock, mDrawingState.bgColorLayer->mFlinger->mStateLock);
         mDrawingState.bgColorLayer->reparent(nullptr);
         mDrawingState.bgColorLayer = nullptr;
         return true;
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 200baf0ba1..f0c8ad76f4 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -409,7 +409,7 @@ public:
     virtual ui::LayerStack getLayerStack() const;
     virtual bool setMetadata(const LayerMetadata& data);
     virtual void setChildrenDrawingParent(const sp<Layer>&);
-    virtual bool reparent(const sp<IBinder>& newParentHandle);
+    virtual bool reparent(const sp<IBinder>& newParentHandle) REQUIRES(mFlinger->mStateLock);
     virtual bool setColorTransform(const mat4& matrix);
     virtual mat4 getColorTransform() const;
     virtual bool hasColorTransform() const;
@@ -433,7 +433,8 @@ public:
     virtual bool setSidebandStream(const sp<NativeHandle>& /*sidebandStream*/) { return false; };
     virtual bool setTransactionCompletedListeners(
             const std::vector<sp<CallbackHandle>>& /*handles*/);
-    virtual bool setBackgroundColor(const half3& color, float alpha, ui::Dataspace dataspace);
+    virtual bool setBackgroundColor(const half3& color, float alpha, ui::Dataspace dataspace)
+            REQUIRES(mFlinger->mStateLock);
     virtual bool setColorSpaceAgnostic(const bool agnostic);
     virtual bool setDimmingEnabled(const bool dimmingEnabled);
     virtual bool setFrameRateSelectionPriority(int32_t priority);
@@ -715,13 +716,13 @@ public:
     /*
      * Remove from current state and mark for removal.
      */
-    void removeFromCurrentState();
+    void removeFromCurrentState() REQUIRES(mFlinger->mStateLock);
 
     /*
      * called with the state lock from a binder thread when the layer is
      * removed from the current list to the pending removal list
      */
-    void onRemovedFromCurrentState();
+    void onRemovedFromCurrentState() REQUIRES(mFlinger->mStateLock);
 
     /*
      * Called when the layer is added back to the current state list.
@@ -899,6 +900,9 @@ public:
 
     virtual bool simpleBufferUpdate(const layer_state_t&) const { return false; }
 
+    // Exposed so SurfaceFlinger can assert that it's held
+    const sp<SurfaceFlinger> mFlinger;
+
 protected:
     friend class impl::SurfaceInterceptor;
 
@@ -974,9 +978,6 @@ protected:
      */
     virtual Rect getInputBounds() const;
 
-    // constant
-    sp<SurfaceFlinger> mFlinger;
-
     bool mPremultipliedAlpha{true};
     const std::string mName;
     const std::string mTransactionName{"TX - " + mName};
diff --git a/services/surfaceflinger/MutexUtils.h b/services/surfaceflinger/MutexUtils.h
index f8be6f3b85..58f7cb4c43 100644
--- a/services/surfaceflinger/MutexUtils.h
+++ b/services/surfaceflinger/MutexUtils.h
@@ -50,4 +50,14 @@ struct SCOPED_CAPABILITY TimedLock {
     const status_t status;
 };
 
+// Require, under penalty of compilation failure, that the compiler thinks that a mutex is held.
+#define REQUIRE_MUTEX(expr) ([]() REQUIRES(expr) {})()
+
+// Tell the compiler that we know that a mutex is held.
+#define ASSERT_MUTEX(expr) ([]() ASSERT_CAPABILITY(expr) {})()
+
+// Specify that one mutex is an alias for another.
+// (e.g. SurfaceFlinger::mStateLock and Layer::mFlinger->mStateLock)
+#define MUTEX_ALIAS(held, alias) (REQUIRE_MUTEX(held), ASSERT_MUTEX(alias))
+
 } // namespace android
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 866958de5d..7c3ca4ba3a 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4415,6 +4415,7 @@ uint32_t SurfaceFlinger::setClientStateLocked(const FrameTimelineInfo& frameTime
         }
         return 0;
     }
+    MUTEX_ALIAS(mStateLock, layer->mFlinger->mStateLock);
 
     // Only set by BLAST adapter layers
     if (what & layer_state_t::eProducerDisconnect) {
@@ -7304,6 +7305,7 @@ void SurfaceFlinger::handleLayerCreatedLocked(const LayerCreatedState& state) {
         ALOGD("Layer was destroyed soon after creation %p", state.layer.unsafe_get());
         return;
     }
+    MUTEX_ALIAS(mStateLock, layer->mFlinger->mStateLock);
 
     sp<Layer> parent;
     bool addToRoot = state.addToRoot;
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 3e3830db2a..17541b876f 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -859,7 +859,7 @@ private:
     // this layer meaning it is entirely safe to destroy all
     // resources associated to this layer.
     void onHandleDestroyed(BBinder* handle, sp<Layer>& layer);
-    void markLayerPendingRemovalLocked(const sp<Layer>& layer);
+    void markLayerPendingRemovalLocked(const sp<Layer>& layer) REQUIRES(mStateLock);
 
     // add a layer to SurfaceFlinger
     status_t addClientLayer(const sp<Client>& client, const sp<IBinder>& handle,