Skip to content

fix: stop exceptions from callbacks unwinding through WebRTC - #335

Merged
devopvoid merged 1 commit into
mainfrom
fix/observer-exceptions
Oct 9, 2026
Merged

devopvoid merged 1 commit into
mainfrom
fix/observer-exceptions

Conversation

@devopvoid

@devopvoid devopvoid commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Builds on #334 (now merged), which moved the ReportUncaughtException helper to WebRTCUtils.

Problem

The callbacks WebRTC invokes ended with ExceptionCheck(env), which turns a pending Java exception into a C++ exception. When the application's observer threw, or converting a callback's arguments failed, that exception unwound through WebRTC's frames. WebRTC is built without unwinding support, so this leaves its locks held, which shows up later as a silent hang (the same failure as the earlier LogSink deadlock), or ends the process, depending on the platform.

Fix

Every callback that WebRTC invokes now catches its own C++ exceptions and turns them into Java ones (ThrowCxxJavaException), then ends with the new ReportPendingException(env), which clears the pending exception and hands it to the calling thread's uncaught exception handler, as the JVM does for an exception escaping a Java thread. Converted:

  • PeerConnectionObserver (all callbacks), CreateSessionDescriptionObserver, SetSessionDescriptionObserver
  • RTCDataChannelObserver, RTCDtlsTransportObserver, RTCDtmfSenderObserver, RTCStatsCollectorCallback
  • AudioTrackSink, VideoTrackSink, MediaStreamTrackObserver
  • AudioTransportSink (the audio device's recorded-data sink), DesktopCaptureCallback

The observers from #334 and the data channel send observer, which already reported exceptions this way, now share the helper instead of carrying their own copy.

Converters that throw are unchanged: they run inside JNI calls or callbacks that now catch.

Also fixed in these callbacks

  • AudioTrackSink and AudioTransportSink hold their byte array in a JavaLocalRef, so it is freed when the callback throws (on a native thread a leaked local reference is never reclaimed), and pass jint values to the Java callback instead of size_t through varargs.
  • MediaStreamTrackObserver released the Java track's local reference on every mute or end event instead of deleting it.
  • CustomVideoSource.pushFrame and pushFrameTimestamped let a C++ exception from converting the frame leave the JNI call; they now throw it as a Java exception.

Tests

ObserverExceptionTests makes the stats callback, the create-offer observer, the peer connection observer (onIceCandidate, onConnectionChange) and a data channel observer throw on every call, and checks that each exception reaches the uncaught exception handler, that WebRTC keeps working (the next stats and offer arrive, the peers connect, every message is delivered), and that close() returns within 10 seconds.

The full webrtc suite passes on Windows (253 run, 0 failures, 2 skipped), also under -Pjni-check with no JNI errors.

@devopvoid
devopvoid added this pull request to stack #336 October 9, 2026 22:10
Base automatically changed from feat/peer-connection-api-gaps to main October 9, 2026 22:50
The callbacks WebRTC invokes ended with ExceptionCheck(), which turns a
Java exception thrown by the application into a C++ exception. It then
unwound through WebRTC's frames, which are built without unwinding
support, leaving WebRTC's locks held or ending the process, depending on
the platform. The same held for C++ exceptions thrown while converting a
callback's arguments.

Every such callback now catches its own C++ exceptions, turns them into
Java ones, and ends with ReportPendingException(), which clears the
pending exception and hands it to the thread's uncaught exception
handler: the peer connection, session description, data channel, DTLS,
DTMF and stats observers, the audio and video track sinks, the track
observer, the audio device's recorded-data sink and the desktop capture
callback. The add-ICE-candidate and data channel send observers, which
already reported exceptions this way, now share the helper.

While in these callbacks:
- the audio sinks hold their byte array in a JavaLocalRef, so it is
  freed when the callback throws, and pass jint values to the Java
  callback instead of size_t through varargs;
- the track observer no longer leaks a local reference per event;
- CustomVideoSource.pushFrame catches conversion errors instead of
  letting a C++ exception leave the JNI call.
@devopvoid
devopvoid force-pushed the fix/observer-exceptions branch from b969638 to 0687b83 Compare October 9, 2026 22:50
@devopvoid
devopvoid merged commit cd66bca into main Oct 9, 2026
22 checks passed
@devopvoid
devopvoid deleted the fix/observer-exceptions branch October 9, 2026 23:10
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant