Bug 965871 - Document lock ordering in APZ. r=kats
authorBotond Ballo <botond@mozilla.com>
Mon, 10 Mar 2014 15:55:44 -0400
changeset 173075 8382b9e3a5511ace3003c9a0e61d8b71ec4f47c7
parent 173074 6e71e620511d68fb8d9f42d3f5c957a06b6c6605
child 173076 10360e4519f4eed1e9ce37a01b23867472375cf8
push id40913
push userbballo@mozilla.com
push dateTue, 11 Mar 2014 21:50:24 +0000
treeherdermozilla-inbound@b9982dcb9a00 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 965871 - Document lock ordering in APZ. r=kats
--- a/gfx/layers/composite/APZCTreeManager.h
+++ b/gfx/layers/composite/APZCTreeManager.h
@@ -37,16 +37,32 @@ enum AllowedTouchBehavior {
   UNKNOWN =            1 << 3
 class Layer;
 class AsyncPanZoomController;
 class CompositorParent;
+ * ****************** NOTE ON LOCK ORDERING IN APZ **************************
+ *
+ * There are two kinds of locks used by APZ: APZCTreeManager::mTreeLock
+ * ("the tree lock") and AsyncPanZoomController::mMonitor ("APZC locks").
+ *
+ * To avoid deadlock, we impose a lock ordering between these locks, which is:
+ *
+ *      tree lock -> APZC locks
+ *
+ * The interpretation of the lock ordering is that if lock A precedes lock B
+ * in the ordering sequence, then you must NOT wait on A while holding B.
+ *
+ * **************************************************************************
+ */
  * This class manages the tree of AsyncPanZoomController instances. There is one
  * instance of this class owned by each CompositorParent, and it contains as
  * many AsyncPanZoomController instances as there are scrollable container layers.
  * This class generally lives on the compositor thread, although some functions
  * may be called from other threads as noted; thread safety is ensured internally.
  * The bulk of the work of this class happens as part of the UpdatePanZoomControllerTree
  * function, which is when a layer tree update is received by the compositor.
@@ -298,17 +314,18 @@ private:
                                                       uint64_t aFirstPaintLayersId,
                                                       nsTArray< nsRefPtr<AsyncPanZoomController> >* aApzcsToDestroy);
   /* Whenever walking or mutating the tree rooted at mRootApzc, mTreeLock must be held.
    * This lock does not need to be held while manipulating a single APZC instance in
    * isolation (that is, if its tree pointers are not being accessed or mutated). The
    * lock also needs to be held when accessing the mRootApzc instance variable, as that
-   * is considered part of the APZC tree management state. */
+   * is considered part of the APZC tree management state.
+   * IMPORTANT: See the note about lock ordering at the top of this file. */
   mozilla::Monitor mTreeLock;
   nsRefPtr<AsyncPanZoomController> mRootApzc;
   /* This tracks the APZC that should receive all inputs for the current input event block.
    * This allows touch points to move outside the thing they started on, but still have the
    * touch events delivered to the same initial APZC. This will only ever be touched on the
    * input delivery thread, and so does not require locking.
   nsRefPtr<AsyncPanZoomController> mApzcForInputBlock;
--- a/gfx/layers/ipc/AsyncPanZoomController.h
+++ b/gfx/layers/ipc/AsyncPanZoomController.h
@@ -645,16 +645,17 @@ protected:
   // Both |mFrameMetrics| and |mLastContentPaintMetrics| are protected by the
   // monitor. Do not read from or modify either of them without locking.
   FrameMetrics mFrameMetrics;
   // Protects |mFrameMetrics|, |mLastContentPaintMetrics|, and |mState|.
   // Before manipulating |mFrameMetrics| or |mLastContentPaintMetrics|, the
   // monitor should be held. When setting |mState|, either the SetState()
   // function can be used, or the monitor can be held and then |mState| updated.
+  // IMPORTANT: See the note about lock ordering at the top of APZCTreeManager.h.
   ReentrantMonitor mMonitor;
   // Specifies whether we should use touch-action css property. Initialized from
   // the preferences. This property (in comparison with the global one) simplifies
   // testing apzc with (and without) touch-action property enabled concurrently
   // (e.g. with the gtest framework).
   bool mTouchActionPropertyEnabled;