Repository navigation
fix: dispose a MediaStream from onAddTrack as a view, and return its ID - #343
Merged
Merged
Conversation
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.
SendableMetatype
added a commit
to EduGeyser/webrtc-java
that referenced
this pull request
Oct 10, 2026
Brings in the work upstream added on October 10: the missing peer connection signaling and control calls (devopvoid#334), callback exceptions kept from unwinding through WebRTC (devopvoid#335), first-packet observers, sender and receiver IDs and track volume (devopvoid#338), video processors for camera and desktop frames (devopvoid#342), and fixes to audio device changes, camera hotplug and media stream views (devopvoid#333, devopvoid#337, devopvoid#341, devopvoid#343). Fork-side adaptations: - setAudioPlayout and setAudioRecording reach the voice engine, so the data channels variant leaves them out, like the other audio natives. - The new tests that need audio or video carry the media tag. - The Maven publish action stays deleted.
SendableMetatype
added a commit
to EduGeyser/webrtc-java
that referenced
this pull request
Oct 10, 2026
Brings in the work upstream added on October 10: the missing peer connection signaling and control calls (devopvoid#334), callback exceptions kept from unwinding through WebRTC (devopvoid#335), first-packet observers, sender and receiver IDs and track volume (devopvoid#338), video processors for camera and desktop frames (devopvoid#342), and fixes to audio device changes, camera hotplug and media stream views (devopvoid#333, devopvoid#337, devopvoid#341, devopvoid#343). Fork-side adaptations: - setBitrate, setAudioPlayout and setAudioRecording reach the call and the voice engine, which a factory without a media engine does not create, so the data channels variant leaves them out, like the other media natives. - The new tests that need audio or video carry the media tag. - The Maven publish action stays deleted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
MediaStreamcounterpart of the track views in #341, plus a second bug its test turned up.Disposing a stream from
onAddTrackreleased the peer connection's referencePeerConnectionObserver::OnAddTrackwrapped the remote streams without taking a reference, whileMediaStream.dispose()released one. Disposing a stream handed toonAddTracktook 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.onAddTrackis the only place JavaMediaStreams come from, so every one is a view, anddispose()now only detaches the Java object. It deliberately takes no reference of its own: the examples (and likely most code) never dispose these streams, so a reference per view would keep every remote stream and its tracks alive. The Javadoc ofMediaStream.dispose()and ofonAddTrack'smediaStreamssays so.MediaStream.id()always returned nullIt returned a local reference that its
JavaLocalRefhad already deleted. It now hands the reference over withrelease(). It was the only JNI return in the codebase missing that.Testing
Windows x64:
mvn -pl webrtc-jni,webrtc verify -Pjni-checkbuilds, and thewebrtcsuite passes (266 tests, 0 failures, 2 skipped) with no fatal JNI errors.MediaStreamViewTestsnegotiates two local peer connections, disposes the streams inonAddTrack, renegotiates, and removes the track. With theRelease()put back intodispose(), removing the track crashed the JVM in three runs out of three; before theid()fix the stream ID came back as null.