RFR: 8268339: Upstream: 8267989: Exceptions thrown during upcalls should be handled (Pt. 2) [v5]

[Paul Sandoz]

On Tue, 8 Jun 2021 17:34:58 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Hi,
>> 
>> ~This is part 2 of a two part upstreaming process of the patch mentioned in the subject line. The patch was split into 2 in order to document 2 separate specification changes that arose from a change in the implementation, with 2 separate CSRs. The first patch can be found here: https://github.com/openjdk/jdk/pull/4395~
>> 
>> This patch adds uniform exception handling for exceptions thrown during FLA upcalls. Currently, these exceptions are either handled similarly to the VMs `CATCH` macro, or ignored after which control returns to unsuspecting native code, until control returns to Java, which will then handle the exception. The handling depends on the invocation mode.
>> 
>> Both of these are bad. The former because a stack trace is not printed and instead the VM exits with a fatal error. The latter is bad because an upcall completing abruptly and returning to native code which has no idea an exception occurred is unsafe, in the sense that invariants about the state of the program that the native code depends on might no longer hold.
>> 
>> This patch adds uniform exception handling that replaces both of these cases (see `SharedUtils::handleUncaughtException`), which prints the exception stack trace, and then unconditionally exits the VM, which is the only safe course of action at that point.
>> 
>> Exceptions thrown by upcalls should instead be handled during the upcall itself, for instance by translating the exception into an error code that is then returned to the native caller, which can deal with it appropriately.
>> 
>> See also the original review for panama-foreign: https://github.com/openjdk/panama-foreign/pull/543
>> 
>> Thanks,
>> Jorn
>> 
>> Testing: `jdk_foreign` test suite.
>> 
>> Thanks,
>> Jorn
>
> Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Suggest try/catch Throwable in upcallStub javadoc

I think this approach makes sense given the native code cannot react to the exception, possibly resulting in undefined behavior.

We might be able to better check upcall handles if they declare they throw and reject them, but it's tricky to nail down the exact conditions, so better to defer and spend more time getting it right.
(Perhaps one day we might be able to reflect over code and do more detailed analysis.)

-------------

Marked as reviewed by psandoz (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4396