commit 0f6bcd18b2ef109850c6430fa0d474688571da5f
author Sami Kalliomäki <sakal@webrtc.org> Mon Feb 03 16:09:45 2020 +0100
committer Commit Bot <commit-bot@chromium.org> Tue Feb 04 15:00:05 2020 +0000
tree 4704d4a180070ea07139633b63c6a82ba37b7c89
parent 215963c75937dec3bf150cb6190bc2c3079f377a

Hold a reference to AndroidVideoTrackSource while calling onFrameCaptured.

This makes it safe to deliver frames to the sink from VideoProcessor
even after setSink has been called with null reference without danger
of use after free.

Bug: b/148063550
Change-Id: Ib78f75ac49fc6117f744c55da1a4e671bbdcdf22
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168160
Reviewed-by: Paulina Hensman <phensman@webrtc.org>
Commit-Queue: Sami Kalliomäki <sakal@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30455}

sdk/android/api/org/webrtc/MediaSource.java

diff --git a/sdk/android/api/org/webrtc/MediaSource.java b/sdk/android/api/org/webrtc/MediaSource.java
index 0b19e1a..9245e3e 100644
--- a/sdk/android/api/org/webrtc/MediaSource.java
+++ b/sdk/android/api/org/webrtc/MediaSource.java
@@ -25,9 +25,11 @@
     }
   }
 
+  private final RefCountDelegate refCountDelegate;
   private long nativeSource;
 
   public MediaSource(long nativeSource) {
+    refCountDelegate = new RefCountDelegate(() -> JniCommon.nativeReleaseRef(nativeSource));
     this.nativeSource = nativeSource;
   }
 
@@ -38,7 +40,7 @@
 
   public void dispose() {
     checkMediaSourceExists();
-    JniCommon.nativeReleaseRef(nativeSource);
+    refCountDelegate.release();
     nativeSource = 0;
   }
 
@@ -48,6 +50,20 @@
     return nativeSource;
   }
 
+  /**
+   * Runs code in {@code runnable} holding a reference to the media source. If the object has
+   * already been released, does nothing.
+   */
+  void runWithReference(Runnable runnable) {
+    if (refCountDelegate.safeRetain()) {
+      try {
+        runnable.run();
+      } finally {
+        refCountDelegate.release();
+      }
+    }
+  }
+
   private void checkMediaSourceExists() {
     if (nativeSource == 0) {
       throw new IllegalStateException("MediaSource has been disposed.");

sdk/android/api/org/webrtc/VideoProcessor.java

diff --git a/sdk/android/api/org/webrtc/VideoProcessor.java b/sdk/android/api/org/webrtc/VideoProcessor.java
index 3a89090..19a2b38 100644
--- a/sdk/android/api/org/webrtc/VideoProcessor.java
+++ b/sdk/android/api/org/webrtc/VideoProcessor.java
@@ -54,7 +54,7 @@
 
   /**
    * Set the sink that receives the output from this processor. Null can be passed in to unregister
-   * a sink. After this call returns, no frames should be delivered to an unregistered sink.
+   * a sink.
    */
   void setSink(@Nullable VideoSink sink);
 

sdk/android/api/org/webrtc/VideoSource.java

diff --git a/sdk/android/api/org/webrtc/VideoSource.java b/sdk/android/api/org/webrtc/VideoSource.java
index 6c528fd..b0bffd6 100644
--- a/sdk/android/api/org/webrtc/VideoSource.java
+++ b/sdk/android/api/org/webrtc/VideoSource.java
@@ -135,7 +135,9 @@
       }
       videoProcessor = newVideoProcessor;
       if (newVideoProcessor != null) {
-        newVideoProcessor.setSink(nativeAndroidVideoTrackSource::onFrameCaptured);
+        newVideoProcessor.setSink(
+            (frame)
+                -> runWithReference(() -> nativeAndroidVideoTrackSource.onFrameCaptured(frame)));
         if (isCapturerRunning) {
           newVideoProcessor.onCapturerStarted(/* success= */ true);
         }