Bug 1453863 - Avoid potential deadlock scenario caused by improper lock acquisiting order. r=zjz
authorKartikaya Gupta <kgupta@mozilla.com>
Wed, 18 Apr 2018 12:19:54 -0400
changeset 467871 a12624f2ce5b9d32c0a68ed8ed1ac54f99cffdd2
parent 467870 b49bd215b26e8831015572770ff8ec7dc91e4dad
child 467872 558fef850d425f13465fe7b8ec0fd249d1527981
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerszjz
bugs1453863
milestone61.0a1
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 1453863 - Avoid potential deadlock scenario caused by improper lock acquisiting order. r=zjz
gfx/layers/apz/src/AsyncPanZoomController.cpp
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -2131,41 +2131,45 @@ AdjustDeltaForAllowedScrollDirections(
 nsEventStatus AsyncPanZoomController::OnScrollWheel(const ScrollWheelInput& aEvent)
 {
   // Get the scroll wheel's delta values in parent-layer pixels. But before
   // getting the values, we need to check if it is an auto-dir scroll and if it
   // should be adjusted, if both answers are yes, let's adjust X and Y values
   // first, and then get the delta values in parent-layer pixels based on the
   // adjusted values.
   bool adjustedByAutoDir = false;
+  auto deltaX = aEvent.mDeltaX;
+  auto deltaY = aEvent.mDeltaY;
   ParentLayerPoint delta;
   if (aEvent.IsAutoDir()) {
     // It's an auto-dir scroll, so check if its delta should be adjusted, if so,
     // adjust it.
     RecursiveMutexAutoLock lock(mRecursiveMutex);
-    auto deltaX = aEvent.mDeltaX;
-    auto deltaY = aEvent.mDeltaY;
     bool isRTL = IsContentOfHonouredTargetRightToLeft(aEvent.HonoursRoot());
     APZAutoDirWheelDeltaAdjuster adjuster(deltaX, deltaY, mX, mY, isRTL);
     if (adjuster.ShouldBeAdjusted()) {
       adjuster.Adjust();
-      // If the original delta values have been adjusted, we pass them to
-      // replace the original delta values in |aEvent| so that the delta values
-      // in parent-layer pixels are caculated based on the adjusted values, not
-      // the original ones.
-      // Pay special attention to the last two parameters. They are in a swaped
-      // order so that they still correspond to their delta after adjustment.
-      delta = GetScrollWheelDelta(aEvent,
-                                  deltaX, deltaY,
-                                  aEvent.mUserDeltaMultiplierY,
-                                  aEvent.mUserDeltaMultiplierX);
       adjustedByAutoDir = true;
     }
   }
-  if (!adjustedByAutoDir) {
+  // Ensure the calls to GetScrollWheelDelta are outside the mRecursiveMutex
+  // lock since these calls may acquire the APZ tree lock. Holding mRecursiveMutex
+  // while acquiring the APZ tree lock is lock ordering violation.
+  if (adjustedByAutoDir) {
+    // If the original delta values have been adjusted, we pass them to
+    // replace the original delta values in |aEvent| so that the delta values
+    // in parent-layer pixels are caculated based on the adjusted values, not
+    // the original ones.
+    // Pay special attention to the last two parameters. They are in a swaped
+    // order so that they still correspond to their delta after adjustment.
+    delta = GetScrollWheelDelta(aEvent,
+                                deltaX, deltaY,
+                                aEvent.mUserDeltaMultiplierY,
+                                aEvent.mUserDeltaMultiplierX);
+  } else {
     // If the original delta values haven't been adjusted by auto-dir, just pass
     // the |aEvent| and caculate the delta values in parent-layer pixels based
     // on the original delta values from |aEvent|.
     delta = GetScrollWheelDelta(aEvent);
   }
 
   APZC_LOG("%p got a scroll-wheel with delta in parent-layer pixels: %s\n",
            this, Stringify(delta).c_str());