From 0687b834b3f1509022e0d0700435ed485c0ca879 Mon Sep 17 00:00:00 2001 From: Alex Andres Date: Fri, 9 Oct 2026 23:53:14 +0200 Subject: [PATCH] fix: stop exceptions from callbacks unwinding through WebRTC 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. --- .../cpp/include/api/PeerConnectionObserver.h | 4 - .../src/main/cpp/include/api/WebRTCUtils.h | 8 +- .../main/cpp/src/JNI_CustomVideoSource.cpp | 26 +- .../cpp/src/api/AddIceCandidateObserver.cpp | 6 +- .../src/main/cpp/src/api/AudioTrackSink.cpp | 15 +- .../api/CreateSessionDescriptionObserver.cpp | 23 +- .../cpp/src/api/PeerConnectionObserver.cpp | 134 ++++++---- .../cpp/src/api/RTCDataChannelObserver.cpp | 16 +- .../src/api/RTCDataChannelSendObserver.cpp | 6 +- .../cpp/src/api/RTCDtlsTransportObserver.cpp | 22 +- .../cpp/src/api/RTCDtmfSenderObserver.cpp | 14 +- .../cpp/src/api/RTCStatsCollectorCallback.cpp | 12 +- .../src/api/SetSessionDescriptionObserver.cpp | 13 +- .../src/main/cpp/src/api/VideoTrackSink.cpp | 21 +- .../src/main/cpp/src/api/WebRTCUtils.cpp | 14 +- .../src/media/MediaStreamTrackObserver.cpp | 28 ++- .../src/media/audio/AudioTransportSink.cpp | 17 +- .../video/desktop/DesktopCaptureCallback.cpp | 36 ++- .../onvoid/webrtc/ObserverExceptionTests.java | 235 ++++++++++++++++++ 19 files changed, 510 insertions(+), 140 deletions(-) create mode 100644 webrtc/src/test/java/dev/onvoid/webrtc/ObserverExceptionTests.java diff --git a/webrtc-jni/src/main/cpp/include/api/PeerConnectionObserver.h b/webrtc-jni/src/main/cpp/include/api/PeerConnectionObserver.h index 82ca0833e..a4d05127f 100644 --- a/webrtc-jni/src/main/cpp/include/api/PeerConnectionObserver.h +++ b/webrtc-jni/src/main/cpp/include/api/PeerConnectionObserver.h @@ -72,10 +72,6 @@ namespace jni }; private: - // Reports a Java exception thrown by a callback without letting it - // unwind through WebRTC. - void ReportException(JNIEnv * env) noexcept; - JavaGlobalRef observer; const std::shared_ptr javaClass; diff --git a/webrtc-jni/src/main/cpp/include/api/WebRTCUtils.h b/webrtc-jni/src/main/cpp/include/api/WebRTCUtils.h index f45fe07d0..9735a8bf9 100644 --- a/webrtc-jni/src/main/cpp/include/api/WebRTCUtils.h +++ b/webrtc-jni/src/main/cpp/include/api/WebRTCUtils.h @@ -82,7 +82,13 @@ namespace jni // uncaught exception handler, as the JVM does for an exception escaping a // Java thread. Callbacks that WebRTC invokes use it, because a C++ // exception must not unwind through WebRTC's frames. - void ReportUncaughtException(JNIEnv * env, jthrowable exception); + void ReportUncaughtException(JNIEnv * env, jthrowable exception) noexcept; + + // Clears the Java exception pending on the calling thread, if any, and + // reports it as uncaught. A callback that WebRTC invokes ends with this + // instead of ExceptionCheck(), which throws, after turning any C++ + // exception of its own into a Java one with ThrowCxxJavaException(). + void ReportPendingException(JNIEnv * env) noexcept; } #endif \ No newline at end of file diff --git a/webrtc-jni/src/main/cpp/src/JNI_CustomVideoSource.cpp b/webrtc-jni/src/main/cpp/src/JNI_CustomVideoSource.cpp index a00e7f2f6..b23d11137 100644 --- a/webrtc-jni/src/main/cpp/src/JNI_CustomVideoSource.cpp +++ b/webrtc-jni/src/main/cpp/src/JNI_CustomVideoSource.cpp @@ -67,10 +67,15 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_CustomVideoSource_push CHECK_HANDLE(source); if (javaFrame != nullptr) { - auto frame = jni::JavaLocalRef(env, javaFrame); - webrtc::VideoFrame nativeFrame = jni::VideoFrame::toNative(env, frame); - - source->PushFrame(nativeFrame); + try { + auto frame = jni::JavaLocalRef(env, javaFrame); + webrtc::VideoFrame nativeFrame = jni::VideoFrame::toNative(env, frame); + + source->PushFrame(nativeFrame); + } + catch (...) { + ThrowCxxJavaException(env); + } } } @@ -81,9 +86,14 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_CustomVideoSource_push CHECK_HANDLE(source); if (javaFrame != nullptr) { - auto frame = jni::JavaLocalRef(env, javaFrame); - webrtc::VideoFrame nativeFrame = jni::VideoFrame::toNative(env, frame); - - source->PushFrame(nativeFrame, timestampUs); + try { + auto frame = jni::JavaLocalRef(env, javaFrame); + webrtc::VideoFrame nativeFrame = jni::VideoFrame::toNative(env, frame); + + source->PushFrame(nativeFrame, timestampUs); + } + catch (...) { + ThrowCxxJavaException(env); + } } } diff --git a/webrtc-jni/src/main/cpp/src/api/AddIceCandidateObserver.cpp b/webrtc-jni/src/main/cpp/src/api/AddIceCandidateObserver.cpp index fd257d783..79a24fcbc 100644 --- a/webrtc-jni/src/main/cpp/src/api/AddIceCandidateObserver.cpp +++ b/webrtc-jni/src/main/cpp/src/api/AddIceCandidateObserver.cpp @@ -79,11 +79,7 @@ namespace jni } // A Java exception must not escape into WebRTC's signaling thread. - if (env->ExceptionCheck()) { - JavaLocalRef exception(env, env->ExceptionOccurred()); - env->ExceptionClear(); - ReportUncaughtException(env, exception.get()); - } + ReportPendingException(env); } AddIceCandidateObserver::JavaAddIceCandidateObserverClass::JavaAddIceCandidateObserverClass(JNIEnv * env) diff --git a/webrtc-jni/src/main/cpp/src/api/AudioTrackSink.cpp b/webrtc-jni/src/main/cpp/src/api/AudioTrackSink.cpp index 957668f99..f9451565e 100644 --- a/webrtc-jni/src/main/cpp/src/api/AudioTrackSink.cpp +++ b/webrtc-jni/src/main/cpp/src/api/AudioTrackSink.cpp @@ -15,7 +15,9 @@ */ #include "api/AudioTrackSink.h" +#include "api/WebRTCUtils.h" #include "JavaClasses.h" +#include "JavaUtils.h" #include "JNI_WebRTC.h" namespace jni @@ -37,12 +39,15 @@ namespace jni const jbyte * buffer = static_cast(data); jsize dataSize = static_cast(frames * channels * (bitsPerSample / 8)); - jbyteArray dataArray = env->NewByteArray(dataSize); - env->SetByteArrayRegion(dataArray, 0, dataSize, buffer); + JavaLocalRef dataArray(env, env->NewByteArray(dataSize)); - env->CallVoidMethod(sink, javaClass->onData, dataArray, bitsPerSample, sampleRate, channels, frames); - ExceptionCheck(env); - env->DeleteLocalRef(dataArray); + if (dataArray.get() != nullptr) { + env->SetByteArrayRegion(dataArray.get(), 0, dataSize, buffer); + env->CallVoidMethod(sink, javaClass->onData, dataArray.get(), static_cast(bitsPerSample), + static_cast(sampleRate), static_cast(channels), static_cast(frames)); + } + + ReportPendingException(env); } AudioTrackSink::JavaAudioTrackSinkClass::JavaAudioTrackSinkClass(JNIEnv * env) diff --git a/webrtc-jni/src/main/cpp/src/api/CreateSessionDescriptionObserver.cpp b/webrtc-jni/src/main/cpp/src/api/CreateSessionDescriptionObserver.cpp index 52d897552..61478d3ca 100644 --- a/webrtc-jni/src/main/cpp/src/api/CreateSessionDescriptionObserver.cpp +++ b/webrtc-jni/src/main/cpp/src/api/CreateSessionDescriptionObserver.cpp @@ -18,6 +18,7 @@ #include "api/RTCSessionDescription.h" #include "api/WebRTCUtils.h" #include "JavaString.h" +#include "JavaUtils.h" #include "JNI_WebRTC.h" namespace jni @@ -36,11 +37,16 @@ namespace jni return; } - JavaLocalRef javaDesc = jni::RTCSessionDescription::toJava(env, desc); + try { + JavaLocalRef javaDesc = jni::RTCSessionDescription::toJava(env, desc); - env->CallVoidMethod(observer, javaClass->onSuccess, javaDesc.get()); + env->CallVoidMethod(observer, javaClass->onSuccess, javaDesc.get()); + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); + ReportPendingException(env); } void CreateSessionDescriptionObserver::OnFailure(webrtc::RTCError error) @@ -51,11 +57,16 @@ namespace jni return; } - JavaLocalRef errorMessage = JavaString::toJava(env, RTCErrorToString(error)); + try { + JavaLocalRef errorMessage = JavaString::toJava(env, RTCErrorToString(error)); - env->CallVoidMethod(observer, javaClass->onFailure, errorMessage.get()); + env->CallVoidMethod(observer, javaClass->onFailure, errorMessage.get()); + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); + ReportPendingException(env); } CreateSessionDescriptionObserver::JavaCreateSessionDescObserverClass::JavaCreateSessionDescObserverClass(JNIEnv * env) diff --git a/webrtc-jni/src/main/cpp/src/api/PeerConnectionObserver.cpp b/webrtc-jni/src/main/cpp/src/api/PeerConnectionObserver.cpp index 48b2430b0..1d093e995 100644 --- a/webrtc-jni/src/main/cpp/src/api/PeerConnectionObserver.cpp +++ b/webrtc-jni/src/main/cpp/src/api/PeerConnectionObserver.cpp @@ -45,11 +45,16 @@ namespace jni return; } - auto jState = JavaEnums::toJava(env, state); + try { + auto jState = JavaEnums::toJava(env, state); - env->CallVoidMethod(observer, javaClass->onConnectionChange, jState.get()); + env->CallVoidMethod(observer, javaClass->onConnectionChange, jState.get()); + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); + ReportPendingException(env); } void PeerConnectionObserver::OnSignalingChange(webrtc::PeerConnectionInterface::SignalingState state) @@ -60,11 +65,16 @@ namespace jni return; } - auto jState = JavaEnums::toJava(env, state); + try { + auto jState = JavaEnums::toJava(env, state); - env->CallVoidMethod(observer, javaClass->onSignalingChange, jState.get()); + env->CallVoidMethod(observer, javaClass->onSignalingChange, jState.get()); + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); + ReportPendingException(env); } void PeerConnectionObserver::OnTrack(webrtc::scoped_refptr transceiver) @@ -75,15 +85,20 @@ namespace jni return; } - // transceiver is passed by value, so this call owns one reference on - // it; transfer that reference into the Java wrapper, which disposes - // it, instead of releasing it right back when transceiver goes out - // of scope and leaving the wrapper's pointer unowned. - auto jTransceiver = JavaFactories::create(env, transceiver.release()); + try { + // transceiver is passed by value, so this call owns one reference on + // it; transfer that reference into the Java wrapper, which disposes + // it, instead of releasing it right back when transceiver goes out + // of scope and leaving the wrapper's pointer unowned. + auto jTransceiver = JavaFactories::create(env, transceiver.release()); - env->CallVoidMethod(observer, javaClass->onTrack, jTransceiver.get()); + env->CallVoidMethod(observer, javaClass->onTrack, jTransceiver.get()); + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); + ReportPendingException(env); } void PeerConnectionObserver::OnAddTrack(webrtc::scoped_refptr receiver, const std::vector> & streams) @@ -111,7 +126,7 @@ namespace jni ThrowCxxJavaException(env); } - ExceptionCheck(env); + ReportPendingException(env); } void PeerConnectionObserver::OnRemoveTrack(webrtc::scoped_refptr receiver) @@ -122,15 +137,20 @@ namespace jni return; } - // receiver is passed by value, so this call owns one reference on - // it; transfer that reference into the Java wrapper, which disposes - // it, instead of releasing it right back when receiver goes out of - // scope and leaving the wrapper's pointer unowned. - auto jReceiver = JavaFactories::create(env, receiver.release()); + try { + // receiver is passed by value, so this call owns one reference on + // it; transfer that reference into the Java wrapper, which disposes + // it, instead of releasing it right back when receiver goes out of + // scope and leaving the wrapper's pointer unowned. + auto jReceiver = JavaFactories::create(env, receiver.release()); - env->CallVoidMethod(observer, javaClass->onRemoveTrack, jReceiver.get()); + env->CallVoidMethod(observer, javaClass->onRemoveTrack, jReceiver.get()); + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); + ReportPendingException(env); } void PeerConnectionObserver::OnDataChannel(webrtc::scoped_refptr channel) @@ -141,11 +161,16 @@ namespace jni return; } - auto jDataChannel = JavaFactories::create(env, channel.release()); + try { + auto jDataChannel = JavaFactories::create(env, channel.release()); - env->CallVoidMethod(observer, javaClass->onDataChannel, jDataChannel.get()); + env->CallVoidMethod(observer, javaClass->onDataChannel, jDataChannel.get()); + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); + ReportPendingException(env); } void PeerConnectionObserver::OnRenegotiationNeeded() @@ -158,7 +183,7 @@ namespace jni env->CallVoidMethod(observer, javaClass->onRenegotiationNeeded); - ExceptionCheck(env); + ReportPendingException(env); } void PeerConnectionObserver::OnIceConnectionChange(webrtc::PeerConnectionInterface::IceConnectionState state) @@ -169,11 +194,16 @@ namespace jni return; } - auto jState = JavaEnums::toJava(env, state); + try { + auto jState = JavaEnums::toJava(env, state); - env->CallVoidMethod(observer, javaClass->onIceConnectionChange, jState.get()); + env->CallVoidMethod(observer, javaClass->onIceConnectionChange, jState.get()); + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); + ReportPendingException(env); } void PeerConnectionObserver::OnIceGatheringChange(webrtc::PeerConnectionInterface::IceGatheringState state) @@ -184,11 +214,16 @@ namespace jni return; } - auto jState = JavaEnums::toJava(env, state); + try { + auto jState = JavaEnums::toJava(env, state); - env->CallVoidMethod(observer, javaClass->onIceGatheringChange, jState.get()); + env->CallVoidMethod(observer, javaClass->onIceGatheringChange, jState.get()); + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); + ReportPendingException(env); } void PeerConnectionObserver::OnIceCandidate(const webrtc::IceCandidateInterface * candidate) @@ -199,11 +234,16 @@ namespace jni return; } - JavaLocalRef jCandidate = RTCIceCandidate::toJava(env, candidate); + try { + JavaLocalRef jCandidate = RTCIceCandidate::toJava(env, candidate); - env->CallVoidMethod(observer, javaClass->onIceCandidate, jCandidate.get()); + env->CallVoidMethod(observer, javaClass->onIceCandidate, jCandidate.get()); + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); + ReportPendingException(env); } void PeerConnectionObserver::OnIceCandidateError(const std::string & address, int port, const std::string & url, int error_code, const std::string & error_text) @@ -214,11 +254,16 @@ namespace jni return; } - JavaLocalRef event = RTCPeerConnectionIceErrorEvent::toJava(env, address, port, url, error_code, error_text); + try { + JavaLocalRef event = RTCPeerConnectionIceErrorEvent::toJava(env, address, port, url, error_code, error_text); - env->CallVoidMethod(observer, javaClass->onIceCandidateError, event.get()); + env->CallVoidMethod(observer, javaClass->onIceCandidateError, event.get()); + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); + ReportPendingException(env); } void PeerConnectionObserver::OnIceCandidateRemoved(const webrtc::IceCandidate * candidate) @@ -240,7 +285,7 @@ namespace jni ThrowCxxJavaException(env); } - ReportException(env); + ReportPendingException(env); } void PeerConnectionObserver::OnIceConnectionReceivingChange(bool receiving) @@ -253,7 +298,7 @@ namespace jni env->CallVoidMethod(observer, javaClass->onIceConnectionReceivingChange, receiving); - ExceptionCheck(env); + ReportPendingException(env); } void PeerConnectionObserver::OnIceSelectedCandidatePairChanged(const webrtc::CandidatePairChangeEvent & event) @@ -275,16 +320,7 @@ namespace jni ThrowCxxJavaException(env); } - ReportException(env); - } - - void PeerConnectionObserver::ReportException(JNIEnv * env) noexcept - { - if (env->ExceptionCheck()) { - JavaLocalRef exception(env, env->ExceptionOccurred()); - env->ExceptionClear(); - ReportUncaughtException(env, exception.get()); - } + ReportPendingException(env); } PeerConnectionObserver::JavaPeerConnectionObserverClass::JavaPeerConnectionObserverClass(JNIEnv * env) diff --git a/webrtc-jni/src/main/cpp/src/api/RTCDataChannelObserver.cpp b/webrtc-jni/src/main/cpp/src/api/RTCDataChannelObserver.cpp index 6333bd06d..8e82b8187 100644 --- a/webrtc-jni/src/main/cpp/src/api/RTCDataChannelObserver.cpp +++ b/webrtc-jni/src/main/cpp/src/api/RTCDataChannelObserver.cpp @@ -15,6 +15,7 @@ */ #include "api/RTCDataChannelObserver.h" +#include "api/WebRTCUtils.h" #include "JavaFactories.h" #include "JavaUtils.h" #include "JNI_WebRTC.h" @@ -38,7 +39,7 @@ namespace jni env->CallVoidMethod(observer, javaClass->onStateChange); - ExceptionCheck(env); + ReportPendingException(env); } void RTCDataChannelObserver::OnMessage(const webrtc::DataBuffer & buffer) @@ -49,11 +50,16 @@ namespace jni return; } - JavaLocalRef jBuffer = bufferFactory->create(env, &buffer); + try { + JavaLocalRef jBuffer = bufferFactory->create(env, &buffer); - env->CallVoidMethod(observer, javaClass->onMessage, jBuffer.get()); + env->CallVoidMethod(observer, javaClass->onMessage, jBuffer.get()); + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); + ReportPendingException(env); } void RTCDataChannelObserver::OnBufferedAmountChange(uint64_t sent_data_size) @@ -66,7 +72,7 @@ namespace jni env->CallVoidMethod(observer, javaClass->onBufferedAmountChange, static_cast(sent_data_size)); - ExceptionCheck(env); + ReportPendingException(env); } RTCDataChannelObserver::JavaRTCDataChannelObserverClass::JavaRTCDataChannelObserverClass(JNIEnv * env) diff --git a/webrtc-jni/src/main/cpp/src/api/RTCDataChannelSendObserver.cpp b/webrtc-jni/src/main/cpp/src/api/RTCDataChannelSendObserver.cpp index 023e54e1c..df1999e90 100644 --- a/webrtc-jni/src/main/cpp/src/api/RTCDataChannelSendObserver.cpp +++ b/webrtc-jni/src/main/cpp/src/api/RTCDataChannelSendObserver.cpp @@ -57,11 +57,7 @@ namespace jni } } // A Java exception must not escape into WebRTC's network task or a destructor. - if (env->ExceptionCheck()) { - JavaLocalRef exception(env, env->ExceptionOccurred()); - env->ExceptionClear(); - ReportUncaughtException(env, exception.get()); - } + ReportPendingException(env); } RTCDataChannelSendObserver::JavaSendObserverClass::JavaSendObserverClass(JNIEnv * env) diff --git a/webrtc-jni/src/main/cpp/src/api/RTCDtlsTransportObserver.cpp b/webrtc-jni/src/main/cpp/src/api/RTCDtlsTransportObserver.cpp index f72d350f0..69227b98e 100644 --- a/webrtc-jni/src/main/cpp/src/api/RTCDtlsTransportObserver.cpp +++ b/webrtc-jni/src/main/cpp/src/api/RTCDtlsTransportObserver.cpp @@ -38,11 +38,16 @@ namespace jni return; } - auto state = JavaEnums::toJava(env, info.state()); + try { + auto state = JavaEnums::toJava(env, info.state()); - env->CallVoidMethod(observer, javaClass->onStateChange, state.get()); + env->CallVoidMethod(observer, javaClass->onStateChange, state.get()); + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); + ReportPendingException(env); } void RTCDtlsTransportObserver::OnError(webrtc::RTCError error) @@ -53,11 +58,16 @@ namespace jni return; } - JavaLocalRef errorMessage = JavaString::toJava(env, RTCErrorToString(error)); + try { + JavaLocalRef errorMessage = JavaString::toJava(env, RTCErrorToString(error)); - env->CallVoidMethod(observer, javaClass->onError, errorMessage.get()); + env->CallVoidMethod(observer, javaClass->onError, errorMessage.get()); + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); + ReportPendingException(env); } RTCDtlsTransportObserver::JavaRTCDtlsTransportObserverClass::JavaRTCDtlsTransportObserverClass(JNIEnv * env) diff --git a/webrtc-jni/src/main/cpp/src/api/RTCDtmfSenderObserver.cpp b/webrtc-jni/src/main/cpp/src/api/RTCDtmfSenderObserver.cpp index a0a549608..db6f05a2d 100644 --- a/webrtc-jni/src/main/cpp/src/api/RTCDtmfSenderObserver.cpp +++ b/webrtc-jni/src/main/cpp/src/api/RTCDtmfSenderObserver.cpp @@ -15,6 +15,7 @@ */ #include "api/RTCDtmfSenderObserver.h" +#include "api/WebRTCUtils.h" #include "JavaClasses.h" #include "JavaString.h" #include "JavaUtils.h" @@ -36,12 +37,17 @@ namespace jni return; } - JavaLocalRef jTone = JavaString::toJava(env, tone); - JavaLocalRef jToneBuffer = JavaString::toJava(env, tone_buffer); + try { + JavaLocalRef jTone = JavaString::toJava(env, tone); + JavaLocalRef jToneBuffer = JavaString::toJava(env, tone_buffer); - env->CallVoidMethod(observer, javaClass->onToneChange, jTone.get(), jToneBuffer.get()); + env->CallVoidMethod(observer, javaClass->onToneChange, jTone.get(), jToneBuffer.get()); + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); + ReportPendingException(env); } RTCDtmfSenderObserver::JavaRTCDtmfSenderObserverClass::JavaRTCDtmfSenderObserverClass(JNIEnv * env) diff --git a/webrtc-jni/src/main/cpp/src/api/RTCStatsCollectorCallback.cpp b/webrtc-jni/src/main/cpp/src/api/RTCStatsCollectorCallback.cpp index 5bfe61c04..408d4b9a8 100644 --- a/webrtc-jni/src/main/cpp/src/api/RTCStatsCollectorCallback.cpp +++ b/webrtc-jni/src/main/cpp/src/api/RTCStatsCollectorCallback.cpp @@ -17,6 +17,7 @@ #include "api/RTCStatsCollectorCallback.h" #include "api/RTCStatsReport.h" #include "api/WebRTCUtils.h" +#include "JavaUtils.h" #include "JNI_WebRTC.h" namespace jni @@ -35,11 +36,16 @@ namespace jni return; } - JavaLocalRef javaReport = jni::RTCStatsReport::toJava(env, report); + try { + JavaLocalRef javaReport = jni::RTCStatsReport::toJava(env, report); - env->CallVoidMethod(callback, javaClass->onStatsDelivered, javaReport.get()); + env->CallVoidMethod(callback, javaClass->onStatsDelivered, javaReport.get()); + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); + ReportPendingException(env); } RTCStatsCollectorCallback::JavaRTCStatsCollectorCallbackClass::JavaRTCStatsCollectorCallbackClass(JNIEnv * env) diff --git a/webrtc-jni/src/main/cpp/src/api/SetSessionDescriptionObserver.cpp b/webrtc-jni/src/main/cpp/src/api/SetSessionDescriptionObserver.cpp index 1ea9ac008..28adbb8d0 100644 --- a/webrtc-jni/src/main/cpp/src/api/SetSessionDescriptionObserver.cpp +++ b/webrtc-jni/src/main/cpp/src/api/SetSessionDescriptionObserver.cpp @@ -38,7 +38,7 @@ namespace jni env->CallVoidMethod(observer, javaClass->onSuccess); - ExceptionCheck(env); + ReportPendingException(env); } void SetSessionDescriptionObserver::OnFailure(webrtc::RTCError error) @@ -49,11 +49,16 @@ namespace jni return; } - JavaLocalRef errorMessage = JavaString::toJava(env, RTCErrorToString(error)); + try { + JavaLocalRef errorMessage = JavaString::toJava(env, RTCErrorToString(error)); - env->CallVoidMethod(observer, javaClass->onFailure, errorMessage.get()); + env->CallVoidMethod(observer, javaClass->onFailure, errorMessage.get()); + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); + ReportPendingException(env); } SetSessionDescriptionObserver::JavaSetSessionDescObserverClass::JavaSetSessionDescObserverClass(JNIEnv * env) diff --git a/webrtc-jni/src/main/cpp/src/api/VideoTrackSink.cpp b/webrtc-jni/src/main/cpp/src/api/VideoTrackSink.cpp index 0c343d04b..c6b41ce68 100644 --- a/webrtc-jni/src/main/cpp/src/api/VideoTrackSink.cpp +++ b/webrtc-jni/src/main/cpp/src/api/VideoTrackSink.cpp @@ -15,7 +15,9 @@ */ #include "api/VideoTrackSink.h" +#include "api/WebRTCUtils.h" #include "JavaClasses.h" +#include "JavaUtils.h" #include "JNI_WebRTC.h" #include "api/video/i420_buffer.h" @@ -52,14 +54,19 @@ namespace jni jint rotation = static_cast(frame.rotation()); jlong timestamp = frame.timestamp_us() * webrtc::kNumNanosecsPerMicrosec; - JavaLocalRef jBuffer = I420Buffer::toJava(env, i420BufferCopy); - jobject jFrame = env->NewObject(javaFrameClass->cls, javaFrameClass->ctor, jBuffer.get(), rotation, timestamp); + try { + JavaLocalRef jBuffer = I420Buffer::toJava(env, i420BufferCopy); + JavaLocalRef jFrame(env, env->NewObject(javaFrameClass->cls, javaFrameClass->ctor, jBuffer.get(), rotation, timestamp)); - env->CallVoidMethod(sink, javaClass->onFrame, jFrame); - ExceptionCheck(env); - // jBuffer is a JavaLocalRef and deletes its own local reference on - // scope exit; deleting it again here would be a double free. - env->DeleteLocalRef(jFrame); + if (jFrame.get() != nullptr) { + env->CallVoidMethod(sink, javaClass->onFrame, jFrame.get()); + } + } + catch (...) { + ThrowCxxJavaException(env); + } + + ReportPendingException(env); } VideoTrackSink::JavaVideoTrackSinkClass::JavaVideoTrackSinkClass(JNIEnv * env) diff --git a/webrtc-jni/src/main/cpp/src/api/WebRTCUtils.cpp b/webrtc-jni/src/main/cpp/src/api/WebRTCUtils.cpp index 2d3b4a250..420e7640f 100644 --- a/webrtc-jni/src/main/cpp/src/api/WebRTCUtils.cpp +++ b/webrtc-jni/src/main/cpp/src/api/WebRTCUtils.cpp @@ -30,7 +30,7 @@ namespace jni // ExceptionDescribe would print the "Exception in thread" prefix straight to // native stderr and the stack trace to System.err, splitting the report // across two streams and leaving the prefix on an unterminated line. - void ReportUncaughtException(JNIEnv * env, jthrowable exception) + void ReportUncaughtException(JNIEnv * env, jthrowable exception) noexcept { JavaLocalRef threadClass(env, env->FindClass("java/lang/Thread")); JavaLocalRef handlerClass(env, env->FindClass("java/lang/Thread$UncaughtExceptionHandler")); @@ -59,4 +59,16 @@ namespace jni env->ExceptionClear(); } } + + void ReportPendingException(JNIEnv * env) noexcept + { + if (!env->ExceptionCheck()) { + return; + } + + JavaLocalRef exception(env, env->ExceptionOccurred()); + env->ExceptionClear(); + + ReportUncaughtException(env, exception.get()); + } } diff --git a/webrtc-jni/src/main/cpp/src/media/MediaStreamTrackObserver.cpp b/webrtc-jni/src/main/cpp/src/media/MediaStreamTrackObserver.cpp index faba1f5dc..dcf36809b 100644 --- a/webrtc-jni/src/main/cpp/src/media/MediaStreamTrackObserver.cpp +++ b/webrtc-jni/src/main/cpp/src/media/MediaStreamTrackObserver.cpp @@ -15,6 +15,7 @@ */ #include "media/MediaStreamTrackObserver.h" +#include "api/WebRTCUtils.h" #include "JavaFactories.h" #include "JavaUtils.h" #include "JNI_WebRTC.h" @@ -41,19 +42,26 @@ namespace jni // Check state changes. - if (eventType == MediaStreamTrackEvent::mute) { - if (track->enabled() != trackEnabled) { - // The condition "muted" is managed using the "enabled" property. - env->CallVoidMethod(javaTrack, javaClass->onTrackMute, createJavaTrack(env).release(), !track->enabled()); - ExceptionCheck(env); + try { + if (eventType == MediaStreamTrackEvent::mute) { + if (track->enabled() != trackEnabled) { + // The condition "muted" is managed using the "enabled" property. + JavaLocalRef jTrack = createJavaTrack(env); + env->CallVoidMethod(javaTrack, javaClass->onTrackMute, jTrack.get(), !track->enabled()); + } } - } - else if (eventType == MediaStreamTrackEvent::ended) { - if (track->state() != trackState && track->state() == webrtc::MediaStreamTrackInterface::TrackState::kEnded) { - env->CallVoidMethod(javaTrack, javaClass->onTrackEnd, createJavaTrack(env).release()); - ExceptionCheck(env); + else if (eventType == MediaStreamTrackEvent::ended) { + if (track->state() != trackState && track->state() == webrtc::MediaStreamTrackInterface::TrackState::kEnded) { + JavaLocalRef jTrack = createJavaTrack(env); + env->CallVoidMethod(javaTrack, javaClass->onTrackEnd, jTrack.get()); + } } } + catch (...) { + ThrowCxxJavaException(env); + } + + ReportPendingException(env); // Save current state. trackEnabled = track->enabled(); diff --git a/webrtc-jni/src/main/cpp/src/media/audio/AudioTransportSink.cpp b/webrtc-jni/src/main/cpp/src/media/audio/AudioTransportSink.cpp index 48fb61709..fffd8c5fc 100644 --- a/webrtc-jni/src/main/cpp/src/media/audio/AudioTransportSink.cpp +++ b/webrtc-jni/src/main/cpp/src/media/audio/AudioTransportSink.cpp @@ -15,7 +15,9 @@ */ #include "media/audio/AudioTransportSink.h" +#include "api/WebRTCUtils.h" #include "JavaClasses.h" +#include "JavaUtils.h" #include "JNI_WebRTC.h" namespace jni @@ -46,12 +48,17 @@ namespace jni const jbyte * buffer = static_cast(audioSamples); jsize dataSize = static_cast(nSamples * nBytesPerSample); - jbyteArray dataArray = env->NewByteArray(dataSize); - env->SetByteArrayRegion(dataArray, 0, dataSize, buffer); - env->CallVoidMethod(sink, javaClass->onRecordedData, dataArray, nSamples, nBytesPerSample, nChannels, samplesPerSec, totalDelayMS, clockDrift); + JavaLocalRef dataArray(env, env->NewByteArray(dataSize)); - ExceptionCheck(env); - env->DeleteLocalRef(dataArray); + if (dataArray.get() != nullptr) { + env->SetByteArrayRegion(dataArray.get(), 0, dataSize, buffer); + env->CallVoidMethod(sink, javaClass->onRecordedData, dataArray.get(), static_cast(nSamples), + static_cast(nBytesPerSample), static_cast(nChannels), static_cast(samplesPerSec), + static_cast(totalDelayMS), static_cast(clockDrift)); + } + + // The audio device's thread must not see a C++ exception. + ReportPendingException(env); return 0; } diff --git a/webrtc-jni/src/main/cpp/src/media/video/desktop/DesktopCaptureCallback.cpp b/webrtc-jni/src/main/cpp/src/media/video/desktop/DesktopCaptureCallback.cpp index 2abebde34..ea0931c31 100644 --- a/webrtc-jni/src/main/cpp/src/media/video/desktop/DesktopCaptureCallback.cpp +++ b/webrtc-jni/src/main/cpp/src/media/video/desktop/DesktopCaptureCallback.cpp @@ -16,8 +16,10 @@ #include "media/video/desktop/DesktopCaptureCallback.h" #include "media/video/desktop/DesktopFrame.h" +#include "api/WebRTCUtils.h" #include "JavaClasses.h" #include "JavaEnums.h" +#include "JavaUtils.h" #include "JNI_WebRTC.h" #include "libyuv/convert.h" @@ -50,14 +52,18 @@ namespace jni if (result != webrtc::DesktopCapturer::Result::SUCCESS) { // Propagate the failure to Java instead of silently dropping it — // callers waiting for a frame otherwise have to rely on timeouts. - auto jerror = JavaEnums::toJava(env, result); - env->CallVoidMethod(callback, javaClass->onCaptureResult, jerror.get(), nullptr); - ExceptionCheck(env); + try { + auto jerror = JavaEnums::toJava(env, result); + env->CallVoidMethod(callback, javaClass->onCaptureResult, jerror.get(), nullptr); + } + catch (...) { + ThrowCxxJavaException(env); + } + + ReportPendingException(env); return; } - auto jresult = JavaEnums::toJava(env, result); - int width = frame->size().width(); int height = frame->size().height(); @@ -113,15 +119,21 @@ namespace jni webrtc::scoped_refptr i420BufferCopy = webrtc::I420Buffer::Copy(*i420Buffer); i420BufferCopy->AddRef(); - JavaLocalRef jBuffer = I420Buffer::toJava(env, i420BufferCopy); - jobject jFrame = env->NewObject(javaFrameClass->cls, javaFrameClass->ctor, jBuffer.get(), rotation, timestamp); + try { + auto jresult = JavaEnums::toJava(env, result); + JavaLocalRef jBuffer = I420Buffer::toJava(env, i420BufferCopy); + JavaLocalRef jFrame(env, env->NewObject(javaFrameClass->cls, javaFrameClass->ctor, jBuffer.get(), rotation, timestamp)); - env->CallVoidMethod(callback, javaClass->onCaptureResult, jresult.get(), jFrame); + if (jFrame.get() != nullptr) { + env->CallVoidMethod(callback, javaClass->onCaptureResult, jresult.get(), jFrame.get()); + } + } + catch (...) { + ThrowCxxJavaException(env); + } - ExceptionCheck(env); - // jBuffer is a JavaLocalRef and deletes its own local reference on - // scope exit; deleting it again here would be a double free. - env->DeleteLocalRef(jFrame); + // The capturer is WebRTC code, whichever thread it calls back on. + ReportPendingException(env); } DesktopCaptureCallback::JavaDesktopCaptureCallbackClass::JavaDesktopCaptureCallbackClass(JNIEnv * env) diff --git a/webrtc/src/test/java/dev/onvoid/webrtc/ObserverExceptionTests.java b/webrtc/src/test/java/dev/onvoid/webrtc/ObserverExceptionTests.java new file mode 100644 index 000000000..5480b5925 --- /dev/null +++ b/webrtc/src/test/java/dev/onvoid/webrtc/ObserverExceptionTests.java @@ -0,0 +1,235 @@ +/* + * 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.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * An exception thrown by an application callback that WebRTC invokes used to + * be rethrown as a C++ exception through WebRTC's frames, which leaks WebRTC's + * locks or ends the process. It now goes to the thread's uncaught exception + * handler, and WebRTC carries on. + */ +class ObserverExceptionTests extends TestBase { + + private final List uncaught = new CopyOnWriteArrayList<>(); + + private Thread.UncaughtExceptionHandler previousHandler; + + + @BeforeEach + void captureUncaught() { + uncaught.clear(); + previousHandler = Thread.getDefaultUncaughtExceptionHandler(); + Thread.setDefaultUncaughtExceptionHandler((thread, error) -> uncaught.add(error)); + } + + @AfterEach + void restoreHandler() { + Thread.setDefaultUncaughtExceptionHandler(previousHandler); + } + + @Test + void statsCallbackThrows() throws Exception { + RTCPeerConnection connection = factory.createPeerConnection(new RTCConfiguration(), candidate -> { }); + + try { + connection.getStats(report -> { + throw new CallbackFailure("stats"); + }); + + awaitUncaught("stats"); + + // The connection still delivers stats. + CountDownLatch delivered = new CountDownLatch(1); + connection.getStats(report -> delivered.countDown()); + + assertTrue(delivered.await(5, TimeUnit.SECONDS)); + } + finally { + closeWithin(connection); + } + } + + @Test + void createOfferObserverThrows() throws Exception { + RTCPeerConnection connection = factory.createPeerConnection(new RTCConfiguration(), candidate -> { }); + + try { + connection.createDataChannel("data", new RTCDataChannelInit()); + connection.createOffer(new RTCOfferOptions(), new CreateSessionDescriptionObserver() { + @Override + public void onSuccess(RTCSessionDescription description) { + throw new CallbackFailure("offer"); + } + + @Override + public void onFailure(String error) { + } + }); + + awaitUncaught("offer"); + + // The signaling thread still runs the next operation. + TestCreateDescObserver observer = new TestCreateDescObserver(); + connection.createOffer(new RTCOfferOptions(), observer); + + assertNotNull(observer.get(5, TimeUnit.SECONDS)); + } + finally { + closeWithin(connection); + } + } + + @Test + void peerConnectionObserverThrows() throws Exception { + RTCPeerConnection[] peers = new RTCPeerConnection[2]; + CountDownLatch connected = new CountDownLatch(2); + + for (int i = 0; i < 2; i++) { + int remote = 1 - i; + + peers[i] = factory.createPeerConnection(new RTCConfiguration(), new PeerConnectionObserver() { + @Override + public void onIceCandidate(RTCIceCandidate candidate) { + peers[remote].addIceCandidate(candidate); + + throw new CallbackFailure("candidate"); + } + + @Override + public void onConnectionChange(RTCPeerConnectionState state) { + if (state == RTCPeerConnectionState.CONNECTED) { + connected.countDown(); + } + + throw new CallbackFailure("connection"); + } + }); + } + + try { + peers[0].createDataChannel("data", new RTCDataChannelInit()); + + TestSetDescObserver setOffer = new TestSetDescObserver(); + peers[0].setLocalDescription(setOffer); + setOffer.get(5, TimeUnit.SECONDS); + + TestSetDescObserver setRemoteOffer = new TestSetDescObserver(); + peers[1].setRemoteDescription(peers[0].getLocalDescription(), setRemoteOffer); + setRemoteOffer.get(5, TimeUnit.SECONDS); + + TestSetDescObserver setAnswer = new TestSetDescObserver(); + peers[1].setLocalDescription(setAnswer); + setAnswer.get(5, TimeUnit.SECONDS); + + TestSetDescObserver setRemoteAnswer = new TestSetDescObserver(); + peers[0].setRemoteDescription(peers[1].getLocalDescription(), setRemoteAnswer); + setRemoteAnswer.get(5, TimeUnit.SECONDS); + + assertTrue(connected.await(10, TimeUnit.SECONDS), "Peers did not connect"); + + awaitUncaught("candidate"); + awaitUncaught("connection"); + } + finally { + // Used to hang on a lock that a thrown callback left held. + closeWithin(peers[0]); + closeWithin(peers[1]); + } + } + + @Test + void dataChannelObserverThrows() throws Exception { + try (TestDataChannelPair pair = new TestDataChannelPair(factory)) { + pair.connect(); + + CountDownLatch received = new CountDownLatch(2); + + pair.receiver.unregisterObserver(); + pair.receiver.registerObserver(new RTCDataChannelObserver() { + @Override + public void onBufferedAmountChange(long previousAmount) { + } + + @Override + public void onStateChange() { + } + + @Override + public void onMessage(RTCDataChannelBuffer buffer) { + received.countDown(); + + throw new CallbackFailure("message"); + } + }); + + // Every message arrives although the observer throws on each one. + for (String text : new String[] { "one", "two" }) { + ByteBuffer data = ByteBuffer.wrap(text.getBytes(StandardCharsets.UTF_8)); + pair.sender.send(new RTCDataChannelBuffer(data, false)); + } + + assertTrue(received.await(5, TimeUnit.SECONDS), "Messages were lost"); + + awaitUncaught("message"); + } + } + + private void awaitUncaught(String callback) throws InterruptedException { + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + + while (System.nanoTime() < deadline) { + for (Throwable error : uncaught) { + if (error instanceof CallbackFailure && callback.equals(error.getMessage())) { + return; + } + } + + Thread.sleep(10); + } + + throw new AssertionError("No uncaught exception from the " + callback + " callback, got " + uncaught); + } + + private static void closeWithin(RTCPeerConnection connection) { + assertTimeoutPreemptively(Duration.ofSeconds(10), connection::close); + } + + + + private static class CallbackFailure extends RuntimeException { + + CallbackFailure(String callback) { + super(callback); + } + } +}