On Mon, 7 Jun 2021 16:38:01 GMT, Jorn Vernee <[email protected]> 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 src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/CABI.java line 43: > 41: static { > 42: String arch = privilegedGetProperty("os.arch"); > 43: String os = privilegedGetProperty("os.name"); These calls were causing problems for the test with a `SecurityManager`. They should have been using `priviledgedGetProperty` in the first place, so I've changed them here. ------------- PR: https://git.openjdk.java.net/jdk/pull/4396
