On Wed, 9 Oct 2024 17:23:08 GMT, Justin Lu <j...@openjdk.org> wrote: >> Switch to `ExceptionCheck`. >> >> This is a part of an umbrella bug [JDK-8341542 JNI uses of >> ExceptionOccurred() treated as if function returns a >> bool](https://bugs.openjdk.org/browse/JDK-8341542). > > src/java.security.jgss/macosx/native/libosxkrb5/SCDynamicStoreConfig.m line > 54: > >> 52: CHECK_NULL(jm_Config_refresh); >> 53: (*env)->CallStaticVoidMethod(env, jc_Config, jm_Config_refresh); >> 54: if ((*env)->ExceptionOccurred(env) != NULL) { > > I believe we can keep the original behavior if you want to, because its being > compared to `NULL` and not as a boolean. > >> Returns the exception object that is currently in the process of being >> thrown, or NULL if no exception is currently being thrown. > > But the new code might read better. So I guess it is just preference.
One concern pointed out by the umbrella bug is that ExceptionOccured(env) returns a jthrowable reference that should be freed by DeleteLocalRef(jthrowable). Even if the reference could be automatically freed later, it's probably still a good practice to either free them early or just not create them at all. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21424#discussion_r1794148140
