From fceb93d5f3effb7655f081eac51c11a58f26fc93 Mon Sep 17 00:00:00 2001 From: Alex Andres Date: Sat, 10 Oct 2026 15:07:37 +0200 Subject: [PATCH] fix: dispose a MediaStream from onAddTrack as a view, and return its ID PeerConnectionObserver::OnAddTrack wrapped the remote streams without taking a reference, while MediaStream.dispose() released one. Disposing a stream handed to onAddTrack took the peer connection's reference instead. When the track was later removed, the receiver dropped the stream and freed it while the peer connection still held it, and the process crashed. onAddTrack is the only place Java MediaStreams come from, so every one is a view: dispose() now only detaches the Java object. It does not take a reference of its own, as nothing disposes these streams today, and a reference per view would keep every remote stream and its tracks alive. MediaStream.id() returned a local reference its JavaLocalRef had already deleted, so it always gave null. It now hands the reference over. MediaStreamViewTests disposes the streams in onAddTrack, renegotiates, and removes the track. With the Release() put back into dispose(), removing the track crashed the JVM in three runs out of three; before the id() fix the stream ID came back as null. --- .../src/main/cpp/src/JNI_MediaStream.cpp | 16 +- .../onvoid/webrtc/PeerConnectionObserver.java | 3 +- .../dev/onvoid/webrtc/media/MediaStream.java | 9 + .../onvoid/webrtc/MediaStreamViewTests.java | 166 ++++++++++++++++++ 4 files changed, 184 insertions(+), 10 deletions(-) create mode 100644 webrtc/src/test/java/dev/onvoid/webrtc/MediaStreamViewTests.java diff --git a/webrtc-jni/src/main/cpp/src/JNI_MediaStream.cpp b/webrtc-jni/src/main/cpp/src/JNI_MediaStream.cpp index ce48406b..dbdb7876 100644 --- a/webrtc-jni/src/main/cpp/src/JNI_MediaStream.cpp +++ b/webrtc-jni/src/main/cpp/src/JNI_MediaStream.cpp @@ -31,7 +31,7 @@ JNIEXPORT jstring JNICALL Java_dev_onvoid_webrtc_media_MediaStream_id webrtc::MediaStreamInterface * stream = GetHandle(env, caller); CHECK_HANDLEV(stream, nullptr); - return jni::JavaString::toJava(env, stream->id()); + return jni::JavaString::toJava(env, stream->id()).release(); } JNIEXPORT jobjectArray JNICALL Java_dev_onvoid_webrtc_media_MediaStream_getAudioTracks @@ -110,13 +110,11 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_MediaStream_dispose webrtc::MediaStreamInterface * stream = GetHandle(env, caller); CHECK_HANDLE(stream); - webrtc::RefCountReleaseStatus status = stream->Release(); - - if (status != webrtc::RefCountReleaseStatus::kDroppedLastRef) { - RTC_LOG(LS_WARNING) << "Native object was not deleted. A reference is still around somewhere."; - } - + // Java MediaStreams are only made by PeerConnectionObserver::OnAddTrack, + // as views of the peer connection's remote streams that hold no reference. + // Releasing one here dropped the peer connection's reference instead, and + // could free the stream while WebRTC still used it. Only the Java object + // is detached. A Java MediaStream made with a reference of its own would + // have to be told apart here, as MediaStreamTrack tells its views apart. SetHandle(env, caller, nullptr); - - stream = nullptr; } \ No newline at end of file diff --git a/webrtc/src/main/java/dev/onvoid/webrtc/PeerConnectionObserver.java b/webrtc/src/main/java/dev/onvoid/webrtc/PeerConnectionObserver.java index 3648de92..4ea7ff77 100644 --- a/webrtc/src/main/java/dev/onvoid/webrtc/PeerConnectionObserver.java +++ b/webrtc/src/main/java/dev/onvoid/webrtc/PeerConnectionObserver.java @@ -145,7 +145,8 @@ default void onRenegotiationNeeded() { * * @param receiver The created RTP receiver. * @param mediaStreams Associated remote MediaStreams of the negotiated - * media track. + * media track. They are views of streams the peer + * connection owns; see {@link MediaStream#dispose()}. */ default void onAddTrack(RTCRtpReceiver receiver, MediaStream[] mediaStreams) { } diff --git a/webrtc/src/main/java/dev/onvoid/webrtc/media/MediaStream.java b/webrtc/src/main/java/dev/onvoid/webrtc/media/MediaStream.java index fb3db015..fb5502d7 100644 --- a/webrtc/src/main/java/dev/onvoid/webrtc/media/MediaStream.java +++ b/webrtc/src/main/java/dev/onvoid/webrtc/media/MediaStream.java @@ -73,6 +73,15 @@ private MediaStream() { */ public native void removeTrack(MediaStreamTrack track); + /** + * Detaches this object from its native stream. + *

+ * A MediaStream is handed out by {@link + * dev.onvoid.webrtc.PeerConnectionObserver#onAddTrack} as a view of a + * remote stream that the peer connection owns, and holds no reference to + * it. Disposing it leaves the stream and its tracks as they are. A view is + * valid for as long as the peer connection keeps the stream. + */ @Override public native void dispose(); diff --git a/webrtc/src/test/java/dev/onvoid/webrtc/MediaStreamViewTests.java b/webrtc/src/test/java/dev/onvoid/webrtc/MediaStreamViewTests.java new file mode 100644 index 00000000..76f573d1 --- /dev/null +++ b/webrtc/src/test/java/dev/onvoid/webrtc/MediaStreamViewTests.java @@ -0,0 +1,166 @@ +/* + * Copyright 2026 Alex Andres + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package dev.onvoid.webrtc; + +import static org.junit.jupiter.api.Assertions.*; + +import dev.onvoid.webrtc.media.MediaStream; +import dev.onvoid.webrtc.media.audio.AudioOptions; +import dev.onvoid.webrtc.media.audio.AudioTrack; +import dev.onvoid.webrtc.media.audio.AudioTrackSource; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; + +/** + * The remote streams handed to onAddTrack are views of streams the peer + * connection owns: disposing one must not release the stream. + */ +@Execution(ExecutionMode.SAME_THREAD) +class MediaStreamViewTests extends TestBase { + + private final List streamIds = Collections.synchronizedList(new ArrayList<>()); + + private final List errors = Collections.synchronizedList(new ArrayList<>()); + + private RTCPeerConnection caller; + + private RTCPeerConnection callee; + + private AudioTrackSource audioSource; + + private AudioTrack audioTrack; + + private RTCRtpSender sender; + + + @BeforeEach + void init() { + // TestBase keeps one instance for all tests of the class. + streamIds.clear(); + errors.clear(); + + RTCConfiguration config = new RTCConfiguration(); + + caller = factory.createPeerConnection(config, candidate -> { }); + callee = factory.createPeerConnection(config, new PeerConnectionObserver() { + + @Override + public void onIceCandidate(RTCIceCandidate candidate) { + } + + @Override + public void onAddTrack(RTCRtpReceiver receiver, MediaStream[] mediaStreams) { + try { + for (MediaStream stream : mediaStreams) { + streamIds.add(stream.id()); + + stream.dispose(); + } + + receiver.dispose(); + } + catch (Throwable e) { + errors.add(e); + } + } + }); + + audioSource = factory.createAudioSource(new AudioOptions()); + audioTrack = factory.createAudioTrack("audioTrack", audioSource); + sender = caller.addTrack(audioTrack, Collections.singletonList("stream-0")); + } + + @AfterEach + void dispose() { + // Closing the callee tears down the remote stream, which fails if a + // disposed view released the reference the peer connection holds. + sender.dispose(); + caller.close(); + callee.close(); + audioTrack.dispose(); + audioSource.dispose(); + } + + @Test + void disposeRemoteStreamView() throws Exception { + negotiate(); + + assertTrue(errors.isEmpty(), () -> "Errors: " + errors); + assertEquals(Collections.singletonList("stream-0"), streamIds); + + // A second offer goes over the same remote stream again. + negotiate(); + + assertTrue(errors.isEmpty(), () -> "Errors: " + errors); + } + + @Test + void removeTrackAfterDisposingStreamView() throws Exception { + negotiate(); + + // Removing the track empties the remote stream, which the peer + // connection then takes apart: a stream freed early by a disposed + // view is used after it was freed. + caller.removeTrack(sender); + negotiate(); + + assertTrue(errors.isEmpty(), () -> "Errors: " + errors); + assertEquals(Collections.singletonList("stream-0"), streamIds); + } + + private void negotiate() throws Exception { + RTCSessionDescription offer = createDescription(caller, true); + setRemoteDescription(callee, offer); + + RTCSessionDescription answer = createDescription(callee, false); + setRemoteDescription(caller, answer); + } + + private static RTCSessionDescription createDescription(RTCPeerConnection connection, boolean offer) throws Exception { + TestCreateDescObserver createObserver = new TestCreateDescObserver(); + TestSetDescObserver setObserver = new TestSetDescObserver(); + + if (offer) { + connection.createOffer(new RTCOfferOptions(), createObserver); + } + else { + connection.createAnswer(new RTCAnswerOptions(), createObserver); + } + + RTCSessionDescription description = createObserver.get(); + + connection.setLocalDescription(description, setObserver); + setObserver.get(); + + return description; + } + + private static void setRemoteDescription(RTCPeerConnection connection, RTCSessionDescription description) throws Exception { + TestSetDescObserver setObserver = new TestSetDescObserver(); + + connection.setRemoteDescription(description, setObserver); + setObserver.get(); + } +}