commit 9445c92925e064d0ea16264a8751994e5cc59acc
author Josh Gao <jmgao@fb.com> Thu Sep 08 16:19:29 2022 -0700
committer Ying Wei <whisperwing@google.com> Fri Mar 31 02:53:14 2023 +0000
tree 6c0101944a80caeca269773b4592a657f8818293
parent b1e815fe34f76430e9ef0af3485185860bb2bdb4

SurfaceFlinger: add more thread-safety annotations.

As part of debugging a crash in SurfaceFlinger, we became suspicious of
markLayerPendingRemovalLocked because it wasn't annotated with thread
safety annotations. This ended up being a red herring, because
mStateLock is held on all paths into that function. Add some helpers to
MutexUtils.h to require/assert mutexes, and use them to sprinkle some
thread safety annotations to make things more clear.

Test: m surfaceflinger
Change-Id: Ib48433765bb6ffcb351ec10cd5bc89e254b48545
Merged-In: Ib48433765bb6ffcb351ec10cd5bc89e254b48545

services/surfaceflinger/Layer.h

diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 200baf0..f0c8ad7 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -409,7 +409,7 @@
     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 @@
     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 @@
     /*
      * 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 @@
 
     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 @@
      */
     virtual Rect getInputBounds() const;
 
-    // constant
-    sp<SurfaceFlinger> mFlinger;
-
     bool mPremultipliedAlpha{true};
     const std::string mName;
     const std::string mTransactionName{"TX - " + mName};