jni-rs
jni-rs copied to clipboard
Display Java exception messages in `Error::JavaException` error message
Overview
(Currently a draft PR because this is based on #478, which isn't yet merged.)
This PR fixes #496 by capturing information about Java exceptions in Error::JavaException
, and displaying that information as part of the error message.
It accomplishes this by storing a GlobalRef
to the exception object, then calling its toString()
method (or, if that also throws an exception, getClass().getName()
) to produce an exception message.
Other changes
In addition to the above, this PR makes two other changes, in separate commits.
This PR adds the method AttachGuard::was_already_attached
, which returns true iff the thread was already attached when JavaVM::attach_current_thread
was called. This information is used to decide whether to log a warning about the thread not being attached when the impl Display for JavaException
is invoked. The GlobalRef
destructor is changed to use this new method as well.
This PR makes JNIEnv::exception_occurred
infallibly return Option<JThrowable>
. It wasn't made infallible in #478 like the other exception-checking JNIEnv
methods, and my exception-capturing code needed it to be infallible.
Breaking changes
Error::JavaException
is now a tuple variant instead of a unit variant, which is a breaking change.
Open questions
Should we do this at all? On my machine, this adds an overhead of at least 2µs every time a JNIEnv
call (other than throw
or throw_new
) results in a Java exception being thrown.
Should the exception message be captured eagerly or lazily? This code is currently lazy: it creates a GlobalRef
to the exception object, and only calls its toString()
if Display::fmt
is called. It could be changed to eagerly call toString()
when the exception is thrown instead. Here's how they compare:
- Lazy mode saves 2µs if
Display::fmt
is called, but costs 2µs ifDisplay::fmt
is not called (i.e. the exception is thrown, but the exception message is never displayed, at least not by Rust code). - Lazy mode costs an additional 86µs if
Display::fmt
andDrop::drop
are called on a thread that's not currently attached to the JVM. This is the same problem as dropping aGlobalRef
on an unattached thread, but doubled: once forDisplay::fmt
, once forDrop::drop
.
Since I expect most Java exceptions will never be displayed by Rust code, only passed back to Java, lazy mode seems like the best approach.
Should users be allowed to suppress exception capture? This removes the benefit of seeing exception messages, but reduces overhead to almost zero. If the answer to this question is yes, should it be by a function call, an environment variable, or both?
Am I overthinking this? Wouldn't be the first time I overthought a performance edge case… 😅
Benchmark results
test tests::exception_capture_eager_nostringify_nosuppress ... bench: 4,097 ns/iter (+/- 60)
test tests::exception_capture_eager_nostringify_suppress ... bench: 20 ns/iter (+/- 0)
test tests::exception_capture_eager_stringify_nosuppress ... bench: 4,135 ns/iter (+/- 516)
test tests::exception_capture_eager_stringify_suppress ... bench: 48 ns/iter (+/- 3)
test tests::exception_capture_lazy_nostringify_nosuppress ... bench: 2,195 ns/iter (+/- 117)
test tests::exception_capture_lazy_nostringify_suppress ... bench: 20 ns/iter (+/- 0)
test tests::exception_capture_lazy_stringify_nosuppress ... bench: 6,410 ns/iter (+/- 116)
test tests::exception_capture_lazy_stringify_suppress ... bench: 48 ns/iter (+/- 1)
test tests::exception_capture_send_baseline ... bench: 29,852 ns/iter (+/- 10,037)
test tests::exception_capture_send_withattach ... bench: 116,000 ns/iter (+/- 26,477)
Benchmark function names contain the following keywords:
-
eager
vslazy
:-
eager
: Calls the exception'stoString()
immediately when it is thrown. -
lazy
: Calls the exception'stoString()
only ifDisplay::fmt
is called.
-
-
stringify
vsnostringify
:-
stringify
:Display::fmt
is called. -
nostringify
: TheJavaException
is silently discarded without ever callingDisplay::fmt
.
-
-
suppress
vsnosuppress
:-
suppress
:toString()
is not called andGlobalRef
is not created.Display::fmt
produces a dummy exception message instead of the real one. -
nosuppress
:toString()
and/orGlobalRef
is called/created.Display::fmt
produces the real exception message.
-
-
send_baseline
andsend_withattach
:-
send_baseline
: Sends theJavaException
to another thread using an MPSC channel, receives it back with another MPSC channel, and then stringifies it as inlazy_stringify_nosuppress
. This benchmark is the baseline forsend_withattach
. -
send_withattach
: Sends theJavaException
to another thread using an MPSC channel, which stringifies and drops it, then sends back the resulting string using another MPSC channel. This demonstrates the severe overhead of stringifying and dropping aJavaException
on an unattached thread.
-
Definition of Done
- [x] There are no TODOs left in the code
- [x] Change is covered by automated tests
- [x] The coding guidelines are followed
- [x] Public API has documentation
- [x] User-visible changes are mentioned in the Changelog
- [ ] The continuous integration build passes