diff --git a/java/shared/java/org/signal/libsignal/internal/NativeTesting.kt b/java/shared/java/org/signal/libsignal/internal/NativeTesting.kt index 0fb31cfa5..77ae8076c 100644 --- a/java/shared/java/org/signal/libsignal/internal/NativeTesting.kt +++ b/java/shared/java/org/signal/libsignal/internal/NativeTesting.kt @@ -156,6 +156,8 @@ public object NativeTesting { @JvmStatic public external fun TESTING_FutureThrowsCustomErrorType(asyncRuntime: ObjectHandle): CompletableFuture @JvmStatic + public external fun TESTING_FutureThrowsPoisonErrorType(asyncRuntime: ObjectHandle): CompletableFuture + @JvmStatic public external fun TESTING_InputStreamReadIntoZeroLengthSlice(capsAlphabetInput: InputStream): ByteArray @JvmStatic public external fun TESTING_JoinStringArray(array: Array, joinWith: String): String diff --git a/java/shared/test/java/org/signal/libsignal/internal/FutureTest.java b/java/shared/test/java/org/signal/libsignal/internal/FutureTest.java index f18ac2119..bcf9f374a 100644 --- a/java/shared/test/java/org/signal/libsignal/internal/FutureTest.java +++ b/java/shared/test/java/org/signal/libsignal/internal/FutureTest.java @@ -53,6 +53,20 @@ public class FutureTest { assertTrue(e.getCause() instanceof org.signal.libsignal.internal.TestingException); } + @Test(timeout = 5000) + public void testFutureThrowsInvalidException() throws Exception { + Future future = NativeTesting.TESTING_FutureThrowsPoisonErrorType(ioRuntime); + ExecutionException e = assertThrows(ExecutionException.class, () -> future.get()); + assertTrue(e.getCause() instanceof AssertionError); + // Check the whole message to make sure it includes both the original error and the failure to + // convert it to an exception. (TestingError just makes that feel especially confusing!) + assertEquals( + e.getCause().getMessage(), + "failed to convert error \"TestingError(org.signal.libsignal.internal.GuaranteedNonexistentException)\": " + + "exception in method call 'org.signal.libsignal.internal.GuaranteedNonexistentException': " + + "exception java.lang.NoClassDefFoundError \"org/signal/libsignal/internal/GuaranteedNonexistentException\""); + } + @Test public void testFutureFromRustCancel() { TokioAsyncContext context = new TokioAsyncContext(); diff --git a/rust/bridge/shared/testing/src/convert.rs b/rust/bridge/shared/testing/src/convert.rs index 46dcdcb49..d89dc1dd5 100644 --- a/rust/bridge/shared/testing/src/convert.rs +++ b/rust/bridge/shared/testing/src/convert.rs @@ -255,6 +255,26 @@ async fn TESTING_FutureThrowsCustomErrorType() -> Result<(), CustomErrorType> { std::future::ready(Err(CustomErrorType)).await } +#[cfg(feature = "jni")] +struct PoisonErrorType; + +#[cfg(feature = "jni")] +impl From for crate::jni::SignalJniError { + fn from(PoisonErrorType: PoisonErrorType) -> Self { + crate::jni::TestingError { + exception_class: crate::jni::ClassName( + "org.signal.libsignal.internal.GuaranteedNonexistentException", + ), + } + .into() + } +} + +#[bridge_io(NonSuspendingBackgroundThreadRuntime, ffi = false, node = false)] +async fn TESTING_FutureThrowsPoisonErrorType() -> Result<(), PoisonErrorType> { + std::future::ready(Err(PoisonErrorType)).await +} + #[bridge_fn] fn TESTING_ReturnStringArray() -> Box<[String]> { ["easy", "as", "ABC", "123"] diff --git a/rust/bridge/shared/types/src/jni/chat.rs b/rust/bridge/shared/types/src/jni/chat.rs index 52e9abc5c..683087cde 100644 --- a/rust/bridge/shared/types/src/jni/chat.rs +++ b/rust/bridge/shared/types/src/jni/chat.rs @@ -111,7 +111,7 @@ impl ChatListener for JniChatListener { fn connection_interrupted(&mut self, disconnect_cause: DisconnectCause) { let listener = &self.listener; attach_and_log_on_error(&self.vm, "connection interrupted", move |env| { - let throw_exception = move |env, listener, throwable: JThrowable<'_>| { + let report_to_java = move |env, listener, throwable: JThrowable<'_>| { call_method_checked( env, listener, @@ -122,21 +122,18 @@ impl ChatListener for JniChatListener { }; match disconnect_cause { DisconnectCause::LocalDisconnect => { - throw_exception(env, listener, JObject::null().into())? + report_to_java(env, listener, JObject::null().into())? + } + DisconnectCause::Error(disconnect_cause) => { + let throwable = SignalJniError::from(disconnect_cause).to_throwable(env); + throwable + .and_then(|throwable| report_to_java(env, listener, throwable)) + .unwrap_or_else(|error| { + log::error!( + "failed to call onConnectionInterrupted with cause: {error}" + ); + }); } - DisconnectCause::Error(disconnect_cause) => convert_to_exception( - env, - SignalJniError::from(disconnect_cause), - move |env, throwable, _error| { - throwable - .and_then(|throwable| throw_exception(env, listener, throwable)) - .unwrap_or_else(|error| { - log::error!( - "failed to call onConnectionInterrupted with cause: {error}" - ); - }); - }, - ), }; Ok(()) }); diff --git a/rust/bridge/shared/types/src/jni/error.rs b/rust/bridge/shared/types/src/jni/error.rs index 65d1c49be..f54f11182 100644 --- a/rust/bridge/shared/types/src/jni/error.rs +++ b/rust/bridge/shared/types/src/jni/error.rs @@ -28,16 +28,38 @@ pub struct TestingError { pub(super) struct AllConnectionAttemptsFailed; impl SignalJniError { + #[cold] pub(super) fn to_throwable<'a>( &self, env: &mut JNIEnv<'a>, ) -> Result, BridgeLayerError> { - self.0.to_throwable(env) + self.0.to_throwable_impl(env).or_else(|convert_error| { + // Recover by producing *some* throwable (AssertionError). This is particularly important + // for Futures, which will otherwise hang. However, if this fails, give up and return the + // *original* BridgeLayerError. + try_scoped(|| { + let message = env + .new_string(format!( + "failed to convert error \"{self}\": {convert_error}" + )) + .check_exceptions(env, "JniError::into_throwable")?; + let error_obj = new_instance( + env, + ClassName("java.lang.AssertionError"), + jni_args!((message => java.lang.Object) -> void), + )?; + Ok(error_obj.into()) + }) + .map_err(|_: BridgeLayerError| convert_error) + }) } } pub(super) trait JniError: Debug + Display { - fn to_throwable<'a>(&self, env: &mut JNIEnv<'a>) -> Result, BridgeLayerError>; + fn to_throwable_impl<'a>( + &self, + env: &mut JNIEnv<'a>, + ) -> Result, BridgeLayerError>; } /// Simpler trait that provides a blanket impl of [`JniError`]. @@ -51,7 +73,10 @@ pub(super) trait MessageOnlyExceptionJniError: Debug + Display { } impl JniError for M { - fn to_throwable<'a>(&self, env: &mut JNIEnv<'a>) -> Result, BridgeLayerError> { + fn to_throwable_impl<'a>( + &self, + env: &mut JNIEnv<'a>, + ) -> Result, BridgeLayerError> { let class = self.exception_class(); let throwable = env .new_string(self.to_string()) diff --git a/rust/bridge/shared/types/src/jni/futures.rs b/rust/bridge/shared/types/src/jni/futures.rs index f367e4d44..09ae1d8c0 100644 --- a/rust/bridge/shared/types/src/jni/futures.rs +++ b/rust/bridge/shared/types/src/jni/futures.rs @@ -150,30 +150,29 @@ impl ResultTypeInfo<'a> + std::panic::UnwindSafe, U> ResultReporter let future_for_convert = &future; let stack_elements_for_convert = &future_creation_stack_trace_elements; maybe_error.unwrap_or_else(move |error| { - convert_to_exception(env, error, move |env, throwable, error| { - throwable - .and_then(move |throwable| { - call_method_checked( - env, - &throwable, - "setStackTrace", - jni_args!((stack_elements_for_convert => [java.lang.StackTraceElement]) -> void), - )?; + let throwable = error.to_throwable(env); + throwable + .and_then(move |throwable| { + call_method_checked( + env, + &throwable, + "setStackTrace", + jni_args!((stack_elements_for_convert => [java.lang.StackTraceElement]) -> void), + )?; - _ = call_method_checked( - env, - future_for_convert, - "completeExceptionally", - jni_args!((throwable => java.lang.Throwable) -> boolean), - )?; - Ok(()) - }) - .unwrap_or_else(|completion_error| { - log::error!( - "failed to complete Future with error \"{error}\": {completion_error}" - ); - }); - }) + _ = call_method_checked( + env, + future_for_convert, + "completeExceptionally", + jni_args!((throwable => java.lang.Throwable) -> boolean), + )?; + Ok(()) + }) + .unwrap_or_else(|completion_error| { + log::error!( + "failed to complete Future with error \"{error}\": {completion_error}" + ); + }); }); // Explicitly drop these while the thread is still attached to the JVM. diff --git a/rust/bridge/shared/types/src/jni/mod.rs b/rust/bridge/shared/types/src/jni/mod.rs index c163b29d1..eee922ad0 100644 --- a/rust/bridge/shared/types/src/jni/mod.rs +++ b/rust/bridge/shared/types/src/jni/mod.rs @@ -109,20 +109,11 @@ impl<'a, T> From> for JObject<'a> { } } -#[cold] -fn convert_to_exception<'a, 'env, F>(env: &'a mut JNIEnv<'env>, error: SignalJniError, consume: F) -where - F: 'a + FnOnce(&'a mut JNIEnv<'env>, Result, BridgeLayerError>, SignalJniError), -{ - // This could be inlined, but then we'd have one copy per unique type for - // `F`. That's expensive in terms of code size, so we break out the - // invariant part into a separate function. - let throwable = error.to_throwable(env); - consume(env, throwable, error) -} - impl JniError for BridgeLayerError { - fn to_throwable<'a>(&self, env: &mut JNIEnv<'a>) -> Result, BridgeLayerError> { + fn to_throwable_impl<'a>( + &self, + env: &mut JNIEnv<'a>, + ) -> Result, BridgeLayerError> { let class_name = match self { BridgeLayerError::CallbackException(_callback, exception) => { return env @@ -160,7 +151,10 @@ impl JniError for BridgeLayerError { } impl JniError for IllegalArgumentError { - fn to_throwable<'a>(&self, env: &mut JNIEnv<'a>) -> Result, BridgeLayerError> { + fn to_throwable_impl<'a>( + &self, + env: &mut JNIEnv<'a>, + ) -> Result, BridgeLayerError> { make_single_message_throwable( env, &self.0, @@ -170,7 +164,10 @@ impl JniError for IllegalArgumentError { } impl JniError for SignalProtocolError { - fn to_throwable<'a>(&self, env: &mut JNIEnv<'a>) -> Result, BridgeLayerError> { + fn to_throwable_impl<'a>( + &self, + env: &mut JNIEnv<'a>, + ) -> Result, BridgeLayerError> { fn to_java_string<'env>( env: &mut JNIEnv<'env>, s: impl Into, @@ -317,7 +314,10 @@ impl JniError for SignalProtocolError { } impl JniError for libsignal_protocol::FingerprintError { - fn to_throwable<'a>(&self, env: &mut JNIEnv<'a>) -> Result, BridgeLayerError> { + fn to_throwable_impl<'a>( + &self, + env: &mut JNIEnv<'a>, + ) -> Result, BridgeLayerError> { let class_name = match self { Self::VersionMismatch { theirs, ours } => return new_instance( env, @@ -354,7 +354,10 @@ fn make_single_message_throwable<'a>( } impl JniError for IoError { - fn to_throwable<'a>(&self, env: &mut JNIEnv<'a>) -> Result, BridgeLayerError> { + fn to_throwable_impl<'a>( + &self, + env: &mut JNIEnv<'a>, + ) -> Result, BridgeLayerError> { if self.kind() == std::io::ErrorKind::Other { let thrown_exception = self .get_ref() @@ -374,7 +377,10 @@ impl JniError for IoError { } impl JniError for libsignal_message_backup::ReadError { - fn to_throwable<'a>(&self, env: &mut JNIEnv<'a>) -> Result, BridgeLayerError> { + fn to_throwable_impl<'a>( + &self, + env: &mut JNIEnv<'a>, + ) -> Result, BridgeLayerError> { let Self { error, found_unknown_fields, @@ -571,19 +577,19 @@ mod registration { use super::*; impl JniError for RequestError { - fn to_throwable<'a>( + fn to_throwable_impl<'a>( &self, env: &mut JNIEnv<'a>, ) -> Result, BridgeLayerError> { let message = match self { - RequestError::Other(inner) => return inner.to_throwable(env), + RequestError::Other(inner) => return inner.to_throwable_impl(env), RequestError::Timeout => { - return libsignal_net::chat::SendError::RequestTimedOut.to_throwable(env); + return libsignal_net::chat::SendError::RequestTimedOut.to_throwable_impl(env); } - RequestError::RetryLater(retry_later) => return retry_later.to_throwable(env), + RequestError::RetryLater(retry_later) => return retry_later.to_throwable_impl(env), RequestError::Unexpected { log_safe } => log_safe, RequestError::Challenge(rate_limit_challenge) => { - return rate_limit_challenge.to_throwable(env); + return rate_limit_challenge.to_throwable_impl(env); } RequestError::ServerSideError => &self.to_string(), RequestError::Disconnected(d) => match *d {}, @@ -624,30 +630,30 @@ mod registration { } impl JniError for CreateSessionError { - fn to_throwable<'a>( + fn to_throwable_impl<'a>( &self, env: &mut JNIEnv<'a>, ) -> Result, BridgeLayerError> { match self { - CreateSessionError::InvalidSessionId => InvalidSessionId.to_throwable(env), + CreateSessionError::InvalidSessionId => InvalidSessionId.to_throwable_impl(env), } } } impl JniError for ResumeSessionError { - fn to_throwable<'a>( + fn to_throwable_impl<'a>( &self, env: &mut JNIEnv<'a>, ) -> Result, BridgeLayerError> { match self { - ResumeSessionError::InvalidSessionId => InvalidSessionId.to_throwable(env), + ResumeSessionError::InvalidSessionId => InvalidSessionId.to_throwable_impl(env), ResumeSessionError::SessionNotFound => session_not_found(env, &self.to_string()), } } } impl JniError for UpdateSessionError { - fn to_throwable<'a>( + fn to_throwable_impl<'a>( &self, env: &mut JNIEnv<'a>, ) -> Result, BridgeLayerError> { @@ -662,13 +668,13 @@ mod registration { } impl JniError for RequestVerificationCodeError { - fn to_throwable<'a>( + fn to_throwable_impl<'a>( &self, env: &mut JNIEnv<'a>, ) -> Result, BridgeLayerError> { match self { RequestVerificationCodeError::InvalidSessionId => { - InvalidSessionId.to_throwable(env) + InvalidSessionId.to_throwable_impl(env) } RequestVerificationCodeError::SessionNotFound => { session_not_found(env, &self.to_string()) @@ -706,12 +712,14 @@ mod registration { } impl JniError for SubmitVerificationError { - fn to_throwable<'a>( + fn to_throwable_impl<'a>( &self, env: &mut JNIEnv<'a>, ) -> Result, BridgeLayerError> { match self { - SubmitVerificationError::InvalidSessionId => InvalidSessionId.to_throwable(env), + SubmitVerificationError::InvalidSessionId => { + InvalidSessionId.to_throwable_impl(env) + } SubmitVerificationError::SessionNotFound => { session_not_found(env, &self.to_string()) } @@ -733,7 +741,7 @@ mod registration { } impl JniError for RegisterAccountError { - fn to_throwable<'a>( + fn to_throwable_impl<'a>( &self, env: &mut JNIEnv<'a>, ) -> Result, BridgeLayerError> { @@ -777,10 +785,13 @@ mod registration { } impl JniError for CdsiError { - fn to_throwable<'a>(&self, env: &mut JNIEnv<'a>) -> Result, BridgeLayerError> { + fn to_throwable_impl<'a>( + &self, + env: &mut JNIEnv<'a>, + ) -> Result, BridgeLayerError> { let class = match *self { CdsiError::RateLimited(retry_later) => { - return retry_later.to_throwable(env); + return retry_later.to_throwable_impl(env); } CdsiError::InvalidToken => { ClassName("org.signal.libsignal.net.CdsiInvalidTokenException") @@ -817,9 +828,12 @@ impl MessageOnlyExceptionJniError for InvalidUri { } impl JniError for ChatConnectError { - fn to_throwable<'a>(&self, env: &mut JNIEnv<'a>) -> Result, BridgeLayerError> { + fn to_throwable_impl<'a>( + &self, + env: &mut JNIEnv<'a>, + ) -> Result, BridgeLayerError> { let class = match *self { - ChatConnectError::RetryLater(retry_later) => return retry_later.to_throwable(env), + ChatConnectError::RetryLater(retry_later) => return retry_later.to_throwable_impl(env), ChatConnectError::AppExpired => { ClassName("org.signal.libsignal.net.AppExpiredException") } @@ -862,7 +876,10 @@ impl MessageOnlyExceptionJniError for ChatSendError { } impl JniError for SvrbError { - fn to_throwable<'a>(&self, env: &mut JNIEnv<'a>) -> Result, BridgeLayerError> { + fn to_throwable_impl<'a>( + &self, + env: &mut JNIEnv<'a>, + ) -> Result, BridgeLayerError> { match self { SvrbError::RestoreFailed(tries_remaining) => { let message = env @@ -883,7 +900,7 @@ impl JniError for SvrbError { &self.to_string(), ClassName("org.signal.libsignal.svr.DataMissingException"), ), - SvrbError::AttestationError(inner) => inner.to_throwable(env), + SvrbError::AttestationError(inner) => inner.to_throwable_impl(env), SvrbError::Protocol(_) => make_single_message_throwable( env, &self.to_string(), @@ -896,7 +913,7 @@ impl JniError for SvrbError { &self.to_string(), ClassName("org.signal.libsignal.net.NetworkException"), ), - SvrbError::RateLimited(inner) => inner.to_throwable(env), + SvrbError::RateLimited(inner) => inner.to_throwable_impl(env), SvrbError::PreviousBackupDataInvalid | SvrbError::MetadataInvalid | SvrbError::DecryptionError(_) => make_single_message_throwable( @@ -941,13 +958,19 @@ impl MessageOnlyExceptionJniError for TestingError { } impl JniError for Infallible { - fn to_throwable<'a>(&self, _env: &mut JNIEnv<'a>) -> Result, BridgeLayerError> { + fn to_throwable_impl<'a>( + &self, + _env: &mut JNIEnv<'a>, + ) -> Result, BridgeLayerError> { match *self {} } } impl JniError for RetryLater { - fn to_throwable<'a>(&self, env: &mut JNIEnv<'a>) -> Result, BridgeLayerError> { + fn to_throwable_impl<'a>( + &self, + env: &mut JNIEnv<'a>, + ) -> Result, BridgeLayerError> { let Self { retry_after_seconds, } = self; @@ -961,7 +984,10 @@ impl JniError for RetryLater { } impl JniError for RateLimitChallenge { - fn to_throwable<'a>(&self, env: &mut JNIEnv<'a>) -> Result, BridgeLayerError> { + fn to_throwable_impl<'a>( + &self, + env: &mut JNIEnv<'a>, + ) -> Result, BridgeLayerError> { let Self { token, options } = self; let (message, token) = try_scoped(|| Ok((env.new_string(self.to_string())?, env.new_string(token)?))) @@ -980,16 +1006,19 @@ impl JniError for RateLimitChallenge { } impl JniError for ChatRequestError { - fn to_throwable<'a>(&self, env: &mut JNIEnv<'a>) -> Result, BridgeLayerError> { + fn to_throwable_impl<'a>( + &self, + env: &mut JNIEnv<'a>, + ) -> Result, BridgeLayerError> { match self { ChatRequestError::Timeout => make_single_message_throwable( env, "Request timed out", ClassName("org.signal.libsignal.net.TimeoutException"), ), - ChatRequestError::Disconnected(disconnected) => disconnected.to_throwable(env), - ChatRequestError::RetryLater(retry_later) => retry_later.to_throwable(env), - ChatRequestError::Challenge(challenge) => challenge.to_throwable(env), + ChatRequestError::Disconnected(disconnected) => disconnected.to_throwable_impl(env), + ChatRequestError::RetryLater(retry_later) => retry_later.to_throwable_impl(env), + ChatRequestError::Challenge(challenge) => challenge.to_throwable_impl(env), ChatRequestError::ServerSideError => make_single_message_throwable( env, "Server-side error", @@ -1000,13 +1029,16 @@ impl JniError for ChatRequestError { &format!("Unexpected error: {}", log_safe), ClassName("org.signal.libsignal.net.UnexpectedResponseException"), ), - ChatRequestError::Other(inner) => inner.to_throwable(env), + ChatRequestError::Other(inner) => inner.to_throwable_impl(env), } } } impl JniError for libsignal_net_chat::api::messages::MultiRecipientSendFailure { - fn to_throwable<'a>(&self, env: &mut JNIEnv<'a>) -> Result, BridgeLayerError> { + fn to_throwable_impl<'a>( + &self, + env: &mut JNIEnv<'a>, + ) -> Result, BridgeLayerError> { let message = self.to_string(); match self { Self::Unauthorized => make_single_message_throwable( @@ -1037,7 +1069,7 @@ impl JniError for libsignal_net_chat::api::messages::MultiRecipientSendFailure { /// appropriate Java exception class and thrown. #[cold] fn throw_error(env: &mut JNIEnv, error: SignalJniError) { - convert_to_exception(env, error, |env, throwable, error| match throwable { + match error.to_throwable(env) { Err(failure) => log::error!("failed to create exception for {error}: {failure}"), Ok(throwable) => { let result = env.throw(throwable); @@ -1045,7 +1077,7 @@ fn throw_error(env: &mut JNIEnv, error: SignalJniError) { log::error!("failed to throw exception for {error}: {failure}"); } } - }); + } } #[inline(always)]