From: Mihai Popa <popam@google.com>
Date: Thu, 29 Mar 2018 14:56:17 +0000 (+0100)
Subject: [Magnifier-38] Avoid deadlock causing ANR
X-Git-Tag: android-x86-9.0-r1~153^2~15^2
X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=5d983d2ba930c101072aec11cb328fb498707568;p=android-x86%2Fframeworks-base.git

[Magnifier-38] Avoid deadlock causing ANR

A deadlock between the UI and render threads caused by magnifier is
currently making the running app to become not responding. The deadlock
could happen in the following scenario:
1. The UI thread sets a frame callback and asks mRenderer to sync and
draw the current frame. A draw task is enqueued by RenderProxy in the
C++ code
2. The UI thread starts an InternalPopupWindow#destroy() on the UI
thread and acquires mLock
3. mRenderer#destroy() is called on the UI thread, which enqueues a
destroy task in the C++ code. However, this is implemented
synchronously, so the UI thread will wait until the destroy task is
dequeued and executed
4. Since the draw task was enqueued before the destroy task, this will
be executed and the frame callback will be called on the render thread
5. The frame callback tries to acquire mLock. However, this is held by
the UI thread, so the render thread has to wait for it. At the same
time, the UI thread cannot progress either as it is waiting for its
destroy task to execute.

The CL adds a new lock which is used by the UI thread to mark its
intention to #destroy(), such that the render thread will know about
this before trying to acquire mLock and starving.

Bug: 75276625
Test: manual testing
Test: atest CtsWidgetTestCases:android.widget.cts.MagnifierTest
Change-Id: Iedf2948350fcf8dd9c819c085b31b7ccaf2db7c5
---

diff --git a/core/java/android/widget/Magnifier.java b/core/java/android/widget/Magnifier.java
index c1c5d9d688ed..5eb66999b296 100644
--- a/core/java/android/widget/Magnifier.java
+++ b/core/java/android/widget/Magnifier.java
@@ -393,6 +393,12 @@ public final class Magnifier {
         private int mWindowPositionY;
         private boolean mPendingWindowPositionUpdate;
 
+        // The lock used to synchronize the UI and render threads when a #destroy
+        // is performed on the UI thread and a frame callback on the render thread.
+        // When both mLock and mDestroyLock need to be held at the same time,
+        // mDestroyLock should be acquired before mLock in order to avoid deadlocks.
+        private final Object mDestroyLock = new Object();
+
         InternalPopupWindow(final Context context, final Display display,
                 final Surface parentSurface,
                 final int width, final int height, final float elevation, final float cornerRadius,
@@ -517,9 +523,11 @@ public final class Magnifier {
          * Destroys this instance.
          */
         public void destroy() {
+            synchronized (mDestroyLock) {
+                mSurface.destroy();
+            }
             synchronized (mLock) {
                 mRenderer.destroy();
-                mSurface.destroy();
                 mSurfaceControl.destroy();
                 mSurfaceSession.kill();
                 mBitmapRenderNode.destroy();
@@ -567,21 +575,23 @@ public final class Magnifier {
                     final int pendingY = mWindowPositionY;
 
                     callback = frame -> {
-                        synchronized (mLock) {
+                        synchronized (mDestroyLock) {
                             if (!mSurface.isValid()) {
                                 return;
                             }
-                            mRenderer.setLightCenter(mDisplay, pendingX, pendingY);
-                            // Show or move the window at the content draw frame.
-                            SurfaceControl.openTransaction();
-                            mSurfaceControl.deferTransactionUntil(mSurface, frame);
-                            if (updateWindowPosition) {
-                                mSurfaceControl.setPosition(pendingX, pendingY);
-                            }
-                            if (firstDraw) {
-                                mSurfaceControl.show();
+                            synchronized (mLock) {
+                                mRenderer.setLightCenter(mDisplay, pendingX, pendingY);
+                                // Show or move the window at the content draw frame.
+                                SurfaceControl.openTransaction();
+                                mSurfaceControl.deferTransactionUntil(mSurface, frame);
+                                if (updateWindowPosition) {
+                                    mSurfaceControl.setPosition(pendingX, pendingY);
+                                }
+                                if (firstDraw) {
+                                    mSurfaceControl.show();
+                                }
+                                SurfaceControl.closeTransaction();
                             }
-                            SurfaceControl.closeTransaction();
                         }
                     };
                 } else {