Skip to content

Commit

Permalink
android: fix race condition when disposing tracks
Browse files Browse the repository at this point in the history
Make sure add/removeSink operations run in the WebRTC thread, so it's
safe to dispose and not get an error when concurrently modifying the
sinks container, which happens inside WebRTC.

Fixes: #1257
  • Loading branch information
saghul committed Jan 11, 2023
1 parent 0403c8b commit ef5d728
Showing 1 changed file with 14 additions and 13 deletions.
27 changes: 14 additions & 13 deletions android/src/main/java/com/oney/WebRTCModule/WebRTCView.java
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ private VideoTrack getVideoTrackForStreamURL(String streamURL) {
Log.w(TAG, "No video stream for react tag: " + streamURL);
}
}

return videoTrack;
}

Expand Down Expand Up @@ -339,14 +339,14 @@ protected void onLayout(boolean changed, int l, int t, int r, int b) {
private void removeRendererFromVideoTrack() {
if (rendererAttached) {
if (videoTrack != null) {
// XXX If WebRTCModule#mediaStreamTrackRelease has already been
// invoked on videoTrack, then it is no longer safe to call removeSink
// on the instance, it will throw IllegalStateException.
try {
videoTrack.removeSink(surfaceViewRenderer);
ThreadUtils.submitToExecutor(() -> {
videoTrack.removeSink(surfaceViewRenderer);
}).get();
} catch (Throwable tr) {
// Releasing streams happens in the WebRTC thread, thus we might (briefly) hold
// a reference to a released stream. Just ignore the error and move on.
// XXX If WebRTCModule#mediaStreamTrackRelease has already been
// invoked on videoTrack, then it is no longer safe to call removeSink
// on the instance, it will throw IllegalStateException.
}
}

Expand Down Expand Up @@ -548,14 +548,15 @@ private void tryAddRendererToVideoTrack() {
surfaceViewRendererInstances--;
}

// XXX If WebRTCModule#mediaStreamTrackRelease has already been
// invoked on videoTrack, then it is no longer safe to call addSink
// on the instance, it will throw IllegalStateException.
try {
videoTrack.addSink(surfaceViewRenderer);
ThreadUtils.submitToExecutor(() -> {
videoTrack.addSink(surfaceViewRenderer);
}).get();
} catch (Throwable tr) {
// Releasing streams happens in the WebRTC thread, thus we might (briefly) hold
// a reference to a released stream.
// XXX If WebRTCModule#mediaStreamTrackRelease has already been
// invoked on videoTrack, then it is no longer safe to call addSink
// on the instance, it will throw IllegalStateException.

Log.e(TAG, "Failed to add renderer", tr);

surfaceViewRenderer.release();
Expand Down

0 comments on commit ef5d728

Please sign in to comment.