fix: report Java exceptions from native callbacks in one piece

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
 

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) {

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;
 			}

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)

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;
 

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<jthrowable> exception(env, env->ExceptionOccurred());
+
+			if (exception.get() != nullptr) {
+				env->ExceptionClear();
+
+				JavaLocalRef<jclass> throwableClass(env, env->GetObjectClass(exception.get()));
+				jmethodID printStackTrace = env->GetMethodID(throwableClass.get(), "printStackTrace", "()V");
+
+				if (printStackTrace != nullptr) {
+					env->CallVoidMethod(exception.get(), printStackTrace);
+				}
 				env->ExceptionClear();
 			}
 		}

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

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);
 	}
 }

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);
 	}
 }

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<jthrowable> exception(env, env->ExceptionOccurred());
+		env->ExceptionClear();
+
+		LocalRef<jclass> thread_class(env, env->FindClass("java/lang/Thread"));
+		LocalRef<jclass> 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<jobject> thread(env, env->CallStaticObjectMethod(thread_class.get(), current_thread));
+		LocalRef<jobject> 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),

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()) {

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);
 		}
 	}