commit 0810db3abccb5433587d956236642d425650684d
author Magnus Jedvert <magjed@webrtc.org> Fri Jul 01 13:52:13 2016
committer Magnus Jedvert <magjed@webrtc.org> Fri Jul 01 13:52:25 2016
tree 9c420fc7e8719a5aabd03b8837eb1f4ccdb70e4c
parent d0b1f7073b68c7bd8a55d41db122621f0a3655fe

Android SurfaceViewRenderer: Fix deadlock

Deadlock caused by two methods grabbing two locks in the opposite order:
renderFrame():
  handlerLock
    layoutLock
onMeasure():
  layoutLock
    handlerLock

This CL removs the nested locking to fix the deadlock and make it less
error prone for the future.

BUG=webrtc:6003
R=sakal@webrtc.org

Review URL: https://codereview.webrtc.org/2111933002 .

Cr-Original-Commit-Position: refs/heads/master@{#13364}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 897d932e0b0b81b81393316692d8687e5975a62a

api/android/java/src/org/webrtc/SurfaceViewRenderer.java

diff --git a/api/android/java/src/org/webrtc/SurfaceViewRenderer.java b/api/android/java/src/org/webrtc/SurfaceViewRenderer.java
index c37d247..4b79fe4 100644
--- a/api/android/java/src/org/webrtc/SurfaceViewRenderer.java
+++ b/api/android/java/src/org/webrtc/SurfaceViewRenderer.java
@@ -295,7 +295,6 @@
           VideoRenderer.renderFrameDone(pendingFrame);
         }
         pendingFrame = frame;
-        updateFrameDimensionsAndReportEvents(frame);
         renderThreadHandler.post(renderFrameRunnable);
       }
     }
@@ -321,23 +320,26 @@
   // View layout interface.
   @Override
   protected void onMeasure(int widthSpec, int heightSpec) {
+    final boolean isNewSize;
     synchronized (layoutLock) {
       if (frameWidth == 0 || frameHeight == 0) {
         super.onMeasure(widthSpec, heightSpec);
         return;
       }
       desiredLayoutSize = getDesiredLayoutSize(widthSpec, heightSpec);
-      if (desiredLayoutSize.x != getMeasuredWidth() || desiredLayoutSize.y != getMeasuredHeight()) {
-        // Clear the surface asap before the layout change to avoid stretched video and other
-        // render artifacs. Don't wait for it to finish because the IO thread should never be
-        // blocked, so it's a best-effort attempt.
-        synchronized (handlerLock) {
-          if (renderThreadHandler != null) {
-            renderThreadHandler.postAtFrontOfQueue(makeBlackRunnable);
-          }
+      isNewSize = (desiredLayoutSize.x != getMeasuredWidth()
+          || desiredLayoutSize.y != getMeasuredHeight());
+      setMeasuredDimension(desiredLayoutSize.x, desiredLayoutSize.y);
+    }
+    if (isNewSize) {
+      // Clear the surface asap before the layout change to avoid stretched video and other
+      // render artifacs. Don't wait for it to finish because the IO thread should never be
+      // blocked, so it's a best-effort attempt.
+      synchronized (handlerLock) {
+        if (renderThreadHandler != null) {
+          renderThreadHandler.postAtFrontOfQueue(makeBlackRunnable);
         }
       }
-      setMeasuredDimension(desiredLayoutSize.x, desiredLayoutSize.y);
     }
   }
 
@@ -446,6 +448,7 @@
       frame = pendingFrame;
       pendingFrame = null;
     }
+    updateFrameDimensionsAndReportEvents(frame);
     if (eglBase == null || !eglBase.hasSurface()) {
       Logging.d(TAG, getResourceName() + "No surface to draw on");
       VideoRenderer.renderFrameDone(frame);