SurfaceTextureHelper: Fix startListening()/stopListening() race

SurfaceTextureHelper.startListening() is asynchronous and posts a Runnable to the handler thread. If stopListening() is called before that Runnable is executed, the Runnable will set the listener after stopListening() has been called. Then the next call to startListening() will fail with "SurfaceTextureHelper listener has already been set."

This CL adds a test to reproduce this bug, and a fix.

BUG=5519,b/27677772

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

Cr-Commit-Position: refs/heads/master@{#12030}

webrtc/api/androidtests/src/org/webrtc/SurfaceTextureHelperTest.java

@@ -273,15 +273,12 @@ public final class SurfaceTextureHelperTest extends ActivityTestCase {
   // Helper method to call stopListening() on correct thread.
   private static void stopListeningOnHandlerThread(final SurfaceTextureHelper surfaceTextureHelper)
       throws InterruptedException {
-    final CountDownLatch barrier = new CountDownLatch(1);
+    ThreadUtils.invokeUninterruptibly(surfaceTextureHelper.getHandler(), new Runnable() {
-    surfaceTextureHelper.getHandler().post(new Runnable() {
       @Override
       public void run() {
         surfaceTextureHelper.stopListening();
-        barrier.countDown();
       }
     });
-    barrier.await();
   }
 
   /**
@@ -335,6 +332,45 @@ public final class SurfaceTextureHelperTest extends ActivityTestCase {
     surfaceTextureHelper.dispose();
   }
 
+  /**
+   * Test stopListening() immediately after the SurfaceTextureHelper has been setup on the handler
+   * thread.
+   */
+  @SmallTest
+  public static void testStopListeningImmediatelyOnHandlerThread() throws InterruptedException {
+    final SurfaceTextureHelper surfaceTextureHelper =
+        SurfaceTextureHelper.create(null);
+    final MockTextureListener listener = new MockTextureListener();
+
+    final CountDownLatch stopListeningBarrier = new CountDownLatch(1);
+    final CountDownLatch stopListeningBarrierDone = new CountDownLatch(1);
+    // Start by posting to the handler thread to keep it occupied.
+    surfaceTextureHelper.getHandler().post(new Runnable() {
+      @Override
+      public void run() {
+        ThreadUtils.awaitUninterruptibly(stopListeningBarrier);
+        surfaceTextureHelper.stopListening();
+        stopListeningBarrierDone.countDown();
+      }
+    });
+
+    // startListening() is asynchronous and will post to the occupied handler thread.
+    surfaceTextureHelper.startListening(listener);
+    // Wait for stopListening() to be called on the handler thread.
+    stopListeningBarrier.countDown();
+    stopListeningBarrierDone.await();
+    // Wait until handler thread is idle to try to catch late startListening() call.
+    ThreadUtils.invokeUninterruptibly(surfaceTextureHelper.getHandler(), new Runnable() {
+      @Override
+      public void run() {}
+    });
+    // Previous startListening() call should never have taken place and it should be ok to call it
+    // again.
+    surfaceTextureHelper.startListening(listener);
+
+    surfaceTextureHelper.dispose();
+  }
+
   /**
    * Test calling startListening() with a new listener after stopListening() has been called.
    */

webrtc/api/java/android/org/webrtc/SurfaceTextureHelper.java

@@ -293,6 +293,19 @@ class SurfaceTextureHelper {
   private boolean hasPendingTexture = false;
   private volatile boolean isTextureInUse = false;
   private boolean isQuitting = false;
+  // |pendingListener| is set in setListener() and the runnable is posted to the handler thread.
+  // setListener() is not allowed to be called again before stopListening(), so this is thread safe.
+  private OnTextureFrameAvailableListener pendingListener;
+  final Runnable setListenerRunnable = new Runnable() {
+    @Override
+    public void run() {
+      Logging.d(TAG, "Setting listener to " + pendingListener);
+      listener = pendingListener;
+      pendingListener = null;
+      // May alredy have a pending frame - try delivering it.
+      tryDeliverTextureFrame();
+    }
+  };
 
   private SurfaceTextureHelper(EglBase.Context sharedContext, Handler handler) {
     if (handler.getLooper().getThread() != Thread.currentThread()) {
@@ -332,17 +345,11 @@ class SurfaceTextureHelper {
    * call stopListening() first.
    */
   public void startListening(final OnTextureFrameAvailableListener listener) {
-    if (this.listener != null) {
+    if (this.listener != null || this.pendingListener != null) {
       throw new IllegalStateException("SurfaceTextureHelper listener has already been set.");
     }
-    handler.post(new Runnable() {
+    this.pendingListener = listener;
-      @Override
+    handler.post(setListenerRunnable);
-      public void run() {
-        SurfaceTextureHelper.this.listener = listener;
-        // May alredy have a pending frame - try delivering it.
-        tryDeliverTextureFrame();
-      }
-    });
   }
 
   /**
@@ -354,7 +361,10 @@ class SurfaceTextureHelper {
     if (handler.getLooper().getThread() != Thread.currentThread()) {
       throw new IllegalStateException("Wrong thread.");
     }
+    Logging.d(TAG, "stopListening()");
+    handler.removeCallbacks(setListenerRunnable);
     this.listener = null;
+    this.pendingListener = null;
   }
 
   /**
@@ -401,6 +411,7 @@ class SurfaceTextureHelper {
    * guaranteed to not receive any more onTextureFrameAvailable() after this function returns.
    */
   public void dispose() {
+    Logging.d(TAG, "dispose()");
     if (handler.getLooper().getThread() == Thread.currentThread()) {
       isQuitting = true;
       if (!isTextureInUse) {