diff --git a/docs/guide/advanced/video-codecs.md b/docs/guide/advanced/video-codecs.md index 57f103550..495ff1573 100644 --- a/docs/guide/advanced/video-codecs.md +++ b/docs/guide/advanced/video-codecs.md @@ -281,7 +281,7 @@ class MyDecoder implements VideoDecoder { ## Errors -Whatever an encoder, decoder or factory method throws is logged and treated as `VideoCodecStatus.ERROR`, and a factory that throws from `createEncoder()` or `createDecoder()` creates no codec for that stream. On an error other than `FALLBACK_SOFTWARE` and `UNINITIALIZED`, WebRTC releases the codec and initializes it again. +Whatever an encoder, decoder or factory method throws goes to the calling thread's uncaught exception handler, is logged, and is treated as `VideoCodecStatus.ERROR`, and a factory that throws from `createEncoder()` or `createDecoder()` creates no codec for that stream. On an error other than `FALLBACK_SOFTWARE` and `UNINITIALIZED`, WebRTC releases the codec and initializes it again. ## Related API diff --git a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaUtils.cpp b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaUtils.cpp index 835c3eb63..f0d6734f7 100644 --- a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaUtils.cpp +++ b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaUtils.cpp @@ -22,8 +22,9 @@ bool ExceptionCheck(JNIEnv * env) { if (env->ExceptionCheck()) { + // Not described here: the exception travels on, to be thrown back + // into Java or reported by whoever catches it. jthrowable exception = env->ExceptionOccurred(); - env->ExceptionDescribe(); env->ExceptionClear(); if (exception) { diff --git a/webrtc-jni/src/main/cpp/src/api/RTCEncodedFrameTransformer.cpp b/webrtc-jni/src/main/cpp/src/api/RTCEncodedFrameTransformer.cpp index daf7c6c2e..1d3509d9c 100644 --- a/webrtc-jni/src/main/cpp/src/api/RTCEncodedFrameTransformer.cpp +++ b/webrtc-jni/src/main/cpp/src/api/RTCEncodedFrameTransformer.cpp @@ -15,6 +15,7 @@ */ #include "api/RTCEncodedFrameTransformer.h" +#include "api/WebRTCUtils.h" #include "JavaClasses.h" #include "JavaUtils.h" #include "JNI_WebRTC.h" @@ -52,8 +53,7 @@ namespace jni bool ClearException(JNIEnv * env) { if (env->ExceptionCheck()) { - env->ExceptionDescribe(); - env->ExceptionClear(); + ReportPendingException(env); return true; } diff --git a/webrtc-jni/src/main/cpp/src/media/DeviceChangeListener.cpp b/webrtc-jni/src/main/cpp/src/media/DeviceChangeListener.cpp index a452644ce..48b4cb530 100644 --- a/webrtc-jni/src/main/cpp/src/media/DeviceChangeListener.cpp +++ b/webrtc-jni/src/main/cpp/src/media/DeviceChangeListener.cpp @@ -15,6 +15,7 @@ */ #include "media/DeviceChangeListener.h" +#include "api/WebRTCUtils.h" #include "media/audio/AudioDevice.h" #include "media/video/VideoDevice.h" #include "JavaFactories.h" @@ -79,10 +80,7 @@ namespace jni RTC_LOG(LS_ERROR) << "Device change notification failed"; } - if (env->ExceptionCheck()) { - env->ExceptionDescribe(); - env->ExceptionClear(); - } + ReportPendingException(env); } DeviceChangeListener::JavaDeviceChangeListenerClass::JavaDeviceChangeListenerClass(JNIEnv * env) diff --git a/webrtc-jni/src/main/cpp/src/media/video/codec/VideoCodecUtils.cpp b/webrtc-jni/src/main/cpp/src/media/video/codec/VideoCodecUtils.cpp index a24366109..afae14816 100644 --- a/webrtc-jni/src/main/cpp/src/media/video/codec/VideoCodecUtils.cpp +++ b/webrtc-jni/src/main/cpp/src/media/video/codec/VideoCodecUtils.cpp @@ -15,6 +15,7 @@ */ #include "media/video/codec/VideoCodecUtils.h" +#include "api/WebRTCUtils.h" #include "JavaClasses.h" #include "JavaUtils.h" #include "JNI_WebRTC.h" @@ -32,8 +33,7 @@ namespace jni // Cleared before logging, since a Java log sink cannot be called // with an exception pending. - env->ExceptionDescribe(); - env->ExceptionClear(); + ReportPendingException(env); RTC_LOG(LS_WARNING) << "Java video codec threw in " << context; diff --git a/webrtc-jni/src/main/cpp/src/rtc/LogSink.cpp b/webrtc-jni/src/main/cpp/src/rtc/LogSink.cpp index 355dca32a..42325e005 100644 --- a/webrtc-jni/src/main/cpp/src/rtc/LogSink.cpp +++ b/webrtc-jni/src/main/cpp/src/rtc/LogSink.cpp @@ -58,9 +58,22 @@ namespace jni env->CallVoidMethod(javaSink, javaClass->onLogMessage, jSeverity.get(), jMessage.get()); - // What the sink threw is reported, but not rethrown. - if (env->ExceptionCheck()) { - env->ExceptionDescribe(); + // What the sink threw is printed, but not rethrown. It is not handed + // to the uncaught exception handler, as other callbacks do: one that + // logs would come back here, to the logging lock this thread holds. + // Nor is it printed with ExceptionDescribe, which writes the first + // part to native stderr and the stack trace to System.err. + JavaLocalRef exception(env, env->ExceptionOccurred()); + + if (exception.get() != nullptr) { + env->ExceptionClear(); + + JavaLocalRef throwableClass(env, env->GetObjectClass(exception.get())); + jmethodID printStackTrace = env->GetMethodID(throwableClass.get(), "printStackTrace", "()V"); + + if (printStackTrace != nullptr) { + env->CallVoidMethod(exception.get(), printStackTrace); + } env->ExceptionClear(); } } diff --git a/webrtc-media/src/main/cpp/include/media/JniBoundary.h b/webrtc-media/src/main/cpp/include/media/JniBoundary.h index 30b2a299a..04be62acc 100644 --- a/webrtc-media/src/main/cpp/include/media/JniBoundary.h +++ b/webrtc-media/src/main/cpp/include/media/JniBoundary.h @@ -41,6 +41,13 @@ namespace ffmpeg // RuntimeException. Only to be called from a catch block. void ThrowCurrentException(JNIEnv * env) noexcept; + // Clears the Java exception pending on the calling thread, if any, and + // hands it to that thread's uncaught exception handler, as the JVM does + // for one escaping a Java thread. Unlike ExceptionDescribe, which writes + // its first line to native stderr and the stack trace to System.err, the + // whole report goes to where the handler writes it. + void ReportPendingException(JNIEnv * env) noexcept; + // Runs the body of a JNI entry point, which must not let a C++ exception // reach the VM: one that does ends the process. A C++ exception becomes a // Java one, and the entry point returns `on_error`. Whatever the body diff --git a/webrtc-media/src/main/cpp/src/media/JavaPlayerObserver.cpp b/webrtc-media/src/main/cpp/src/media/JavaPlayerObserver.cpp index 9a99ea3fd..a8756da3c 100644 --- a/webrtc-media/src/main/cpp/src/media/JavaPlayerObserver.cpp +++ b/webrtc-media/src/main/cpp/src/media/JavaPlayerObserver.cpp @@ -175,12 +175,9 @@ namespace ffmpeg void JavaPlayerObserver::ClearPendingException(JNIEnv * env) { - if (env->ExceptionCheck() == JNI_TRUE) { - // There is nowhere to throw this: the thread below is decoding, - // not running Java. Reporting and clearing keeps it from taking - // down the next unrelated JNI call instead. - env->ExceptionDescribe(); - env->ExceptionClear(); - } + // There is nowhere to throw this: the thread below is decoding, + // not running Java. Reporting and clearing keeps it from taking + // down the next unrelated JNI call instead. + ReportPendingException(env); } } diff --git a/webrtc-media/src/main/cpp/src/media/JavaRecorderObserver.cpp b/webrtc-media/src/main/cpp/src/media/JavaRecorderObserver.cpp index a44a46e49..459a5a14f 100644 --- a/webrtc-media/src/main/cpp/src/media/JavaRecorderObserver.cpp +++ b/webrtc-media/src/main/cpp/src/media/JavaRecorderObserver.cpp @@ -175,9 +175,6 @@ namespace ffmpeg void JavaRecorderObserver::ClearPendingException(JNIEnv * env) { - if (env->ExceptionCheck() == JNI_TRUE) { - env->ExceptionDescribe(); - env->ExceptionClear(); - } + ReportPendingException(env); } } diff --git a/webrtc-media/src/main/cpp/src/media/JniBoundary.cpp b/webrtc-media/src/main/cpp/src/media/JniBoundary.cpp index c8ddd9eef..e26e7a63c 100644 --- a/webrtc-media/src/main/cpp/src/media/JniBoundary.cpp +++ b/webrtc-media/src/main/cpp/src/media/JniBoundary.cpp @@ -204,6 +204,47 @@ namespace ffmpeg } } + void ReportPendingException(JNIEnv * env) noexcept + { + if (env->ExceptionCheck() == JNI_FALSE) { + return; + } + + LocalRef exception(env, env->ExceptionOccurred()); + env->ExceptionClear(); + + LocalRef thread_class(env, env->FindClass("java/lang/Thread")); + LocalRef handler_class(env, env->FindClass("java/lang/Thread$UncaughtExceptionHandler")); + + if (env->ExceptionCheck() == JNI_TRUE) { + env->ExceptionClear(); + return; + } + + jmethodID current_thread = env->GetStaticMethodID(thread_class.get(), "currentThread", "()Ljava/lang/Thread;"); + jmethodID get_handler = env->GetMethodID(thread_class.get(), "getUncaughtExceptionHandler", + "()Ljava/lang/Thread$UncaughtExceptionHandler;"); + jmethodID uncaught_exception = env->GetMethodID(handler_class.get(), "uncaughtException", + "(Ljava/lang/Thread;Ljava/lang/Throwable;)V"); + + if (env->ExceptionCheck() == JNI_TRUE) { + env->ExceptionClear(); + return; + } + + LocalRef thread(env, env->CallStaticObjectMethod(thread_class.get(), current_thread)); + LocalRef handler(env, env->CallObjectMethod(thread.get(), get_handler)); + + if (env->ExceptionCheck() == JNI_FALSE && handler.get() != nullptr) { + env->CallVoidMethod(handler.get(), uncaught_exception, thread.get(), exception.get()); + } + + // Like the JVM, ignore an exception thrown by the handler itself. + if (env->ExceptionCheck() == JNI_TRUE) { + env->ExceptionClear(); + } + } + JavaStringChars::JavaStringChars(JNIEnv * env, jstring string) noexcept : env_(env), string_(string), diff --git a/webrtc-media/src/test/java/dev/onvoid/webrtc/media/player/MediaPlayerTest.java b/webrtc-media/src/test/java/dev/onvoid/webrtc/media/player/MediaPlayerTest.java index d02d5353a..9d7ebedfe 100644 --- a/webrtc-media/src/test/java/dev/onvoid/webrtc/media/player/MediaPlayerTest.java +++ b/webrtc-media/src/test/java/dev/onvoid/webrtc/media/player/MediaPlayerTest.java @@ -400,6 +400,47 @@ public void onEndOfStream() { } } + @Test + void reportsWhatTheListenerThrows() throws Exception { + // What the listener throws on the player's thread goes to that + // thread's uncaught exception handler, as it would on a Java thread, + // rather than half to native stderr through ExceptionDescribe. + CountDownLatch reported = new CountDownLatch(1); + Thread.UncaughtExceptionHandler previous = Thread.getDefaultUncaughtExceptionHandler(); + + Thread.setDefaultUncaughtExceptionHandler((thread, e) -> { + if (e instanceof IllegalStateException && "listener fails on purpose".equals(e.getMessage())) { + reported.countDown(); + } + else if (previous != null) { + previous.uncaughtException(thread, e); + } + }); + + try (Sources sources = new Sources()) { + MediaPlayer player = new MediaPlayer(new MediaReader(asset()), + sources.video, sources.audio); + + player.setListener(new MediaPlayerListener() { + + @Override + public void onEndOfStream() { + throw new IllegalStateException("listener fails on purpose"); + } + }); + + player.seek(2_500_000); + player.play(); + + assertTrue(reported.await(10, TimeUnit.SECONDS), "not reported"); + + player.close(); + } + finally { + Thread.setDefaultUncaughtExceptionHandler(previous); + } + } + @Test void closesFromStateChangeOfCommand() throws Exception { try (Sources sources = new Sources()) { diff --git a/webrtc/src/test/java/dev/onvoid/webrtc/VideoCodecFactoryTests.java b/webrtc/src/test/java/dev/onvoid/webrtc/VideoCodecFactoryTests.java index e7bc1ac22..ba487a3ef 100644 --- a/webrtc/src/test/java/dev/onvoid/webrtc/VideoCodecFactoryTests.java +++ b/webrtc/src/test/java/dev/onvoid/webrtc/VideoCodecFactoryTests.java @@ -365,6 +365,21 @@ public VideoEncoder createEncoder(VideoCodecInfo info) { // exception was still pending. Logging.addLogSink(Logging.Severity.WARNING, (severity, message) -> { }); + // What the factory threw goes to the uncaught exception handler of the + // thread that called it, as it would from a Java thread, rather than + // half to native stderr through ExceptionDescribe. + CountDownLatch reported = new CountDownLatch(1); + Thread.UncaughtExceptionHandler previous = Thread.getDefaultUncaughtExceptionHandler(); + + Thread.setDefaultUncaughtExceptionHandler((thread, e) -> { + if (e instanceof IllegalStateException && "createEncoder fails on purpose".equals(e.getMessage())) { + reported.countDown(); + } + else if (previous != null) { + previous.uncaughtException(thread, e); + } + }); + PeerConnectionFactory built = testCodecFactory(encoderFactory, new TestDecoderFactory()); try (TestMediaCall call = new TestMediaCall(built, true, false, TEST_CODEC)) { @@ -379,9 +394,11 @@ public VideoEncoder createEncoder(VideoCodecInfo info) { } assertTrue(encoderFactory.created.get() > 0, "the encoder factory was never asked"); + assertTrue(reported.await(TIMEOUT_SECONDS, TimeUnit.SECONDS), "the failure was not reported"); } finally { built.dispose(); + Thread.setDefaultUncaughtExceptionHandler(previous); } }