On Fri, 18 Dec 2020 16:01:17 GMT, Gerard Ziemski <gziem...@openjdk.org> wrote:
> There seems to be a pattern where we replace `JNFCallObjectMethod()` with a > pair of `CallObjectMethod()` immediately followed by `CHECK_EXCEPTION()` > > There are places where currently in this fix, this pattern is missing the > `CHECK_EXCEPTION()` > > Can we replace `JNFCallObjectMethod()` by a single function call (or macro) > that wraps both `CallObjectMethod()` and `CHECK_EXCEPTION()` into one? > > This would make this change more robust and make it easier to review it. Its is a reasonable idea and I thought about it but on the whole I'd prefer not to do it and insted fix missing cases. Tthere are cases where we are using these macros directly in a native method and then returning to Java. In those case I want to propagate the exception and this suggestion would make that difficult if not impossible. Remember that these aren't likely to do anything whether missing or not to make things better or cause a regression because we should not be generating these exceptions anyway. This is mostly a recommended precautionary pattern "just in case". And nowhere else in the JDK uses macro for this - the CHECK_EXCEPTION and similar patterns are used elsewhere. And code already added by Oracle engineers after the initial macOS port are using the standard JNI calls. For comparison I've been running JDK 15 with -Xcheck:jni and there were a number of places that generated these warnings and I've resolved them here. So I believe we may end up more (theoretically) robust with my changes than we were before. Perhaps they weren't a result of a JNF* call but still it seems improved. ------------- PR: https://git.openjdk.java.net/jdk/pull/1679
