Introduce a no-exceptions mode to JNIPP

[bialpio]

This PR introduces a no-exceptions mode to JNIPP. Some of the downstream consumers of JNIPP are meant to be built with exceptions disabled, but that becomes a bit problematic when their direct or indirect dependency does not support such mode.

Changes:

• introduce new CMake option, JNIPP_USE_EXCEPTION_HANDLING, ON by default
• introduce preprocessor directive, JNIPP_USE_EXCEPTION, in jnipp.h header (set to 1 by default)
• when JNIPP_USE_EXCEPTION is set to 0, all throw exceptions in jnipp.cpp will be replaced by calls to std::terminate(); this is achieved via a cpp-only macro, JNIPP_THROW
• test cases that rely on exceptions being thrown are not compiled at all if JNIPP is compiled with exceptions support turned off
• when built without exceptions on Clang and GCC, both the library and the tests will be compiled with -fno-exceptions switch

Tested as follows:

1. Exceptions mode:

cmake -S . -B out/exceptions -G Ninja -DJNIPP_USE_EXCEPTION_HANDLING=ON
cmake --build out/exceptions/
ctest --test-dir out/exceptions/

2. No-exceptions mode:

cmake -S . -B out/no-exceptions -G Ninja -DJNIPP_USE_EXCEPTION_HANDLING=OFF
cmake --build out/no-exceptions/
ctest --test-dir out/no-exceptions/

[rpavlik]

ah, interesting! I was thinking that we'd add a noexcept overload for the various functions to populate an exception output parameter

[bialpio]

Just to make sure I understood you correctly, are you envisioning something like the following?

// jnipp.cpp:693
method_t Class::getMethod(const char* name, const char* signature, Exception** ex) const // (1)
{
    jmethodID id = env()->GetMethodID(getHandle(), name, signature);

    if (id == nullptr)
        *ex = new NameResolutionException(name); // (2)

    return id;
}

(changes from the original: (1) adds ex parameter, (2) populates it instead of throwing)

First problem that I see with this is that it'd make the callers' responsibility to free the exception object, otherwise we'll leak memory. The other problem is that I think if the caller would like to learn about the specific type of the exception being returned (note NameResolutionException being allocated but Exception in the signature), they'd have to have a bunch of dynamic_cast<>s to try and ask for a specific type, and projects that compile with exceptions disabled also usually compile with RTTI disabled too, so they'd lose this capability.

I think it may be OK to have an out-parameter that is an enum that carries a result code (or maybe a struct with error code and a message?). There is also std::error_code that maybe would work here, but I don't have that much experience with it. One of the problems with this approach is that we will have an issue of ~ doubling (?) the API surface - each throwing method will need to have a nothrow out-parameter variant, which I expect would trickle up everywhere (e.g. handleJavaExceptions() occurs 61 times in the codebase). The other issue I can think of is that we also throw from constructors, and use throwing methods when invoking base class ctor:

// jnipp.cpp:655
Class::Class(const char* name) : Object(findClass(name), DeleteLocalInput) // findClass() can throw
{
}

Exceptions allowed us to avoid creating "zombie objects" (the ctor would throw, the instance wouldn't be created, all is relatively fine) - the no-exceptions mode will not have that luxury...

[rpavlik]

That does make sense. However, my concern is that in things like Chrome that might use this (because of using openxr), they really don't want to terminate just because there was a JNI exception trying to find openxr

I might take a crack at my idea this week. I'll see if I can avoid having dual code paths, just have the thrower delegate to the no-throw. We already have nullable objects, so that's not that big of a deal.

[bialpio]

Sounds good, thanks for taking a look! I think it still makes sense to terminate in JNIPP, but maybe not in the OpenXR loader (depends on the specific situation). With the auto-generated wrappers built on top of JNIPP, I think the reasons why exceptions may be thrown are quite limited (e.g. NameResolutionExceptions should not be raised at all since they signify a programmer error like a typo in the class name / method name?). Maybe just changing the API surface for places that can throw InitializationExceptions is sufficient here?

[rpavlik]

OK, I put up some of my partial work towards an alternate approach here: #43 I didn't get it nearly finished because it's been a little hectic, but hopefully it shows the idea of where I was going. Should be able to turn each throwing function into a non-throwing one and replace it with a call to the non-throwing one that then calls throwException on the exception data.

[bialpio]

Yeah, I think that approach would work, but as I've mentioned, most of the codebase will probably need to be rewritten. There's also the problem with zombie objects (i.e. objects that exist despite ctor wanting to throw an exception) that I think your draft PR does not show how they'd look like (e.g. Class::Class() would set the exc correctly, but the object itself isn't aware that nothing should be called on it anymore; maybe things will work fine because handle_ won't be valid?).

[alcooper91]

Any update on either this or the alternative?