Re: RFR: JDK-8280902 ResourceBundle::getBundle may throw NPE when invoked by JNI code with no caller frame [v5]
> The caller class returned by Reflection::getCallerClass was used to gain > access to it's module in most cases and class loader in one case. I added a > method to translate the caller class to caller module so that the decision of > what module represents the caller with no stack frame is made in a single > place. Calls made to caller.getModule() were replaced with > getCallerModule(caller) which returns the system class loader unnamed module > if the caller is null. > > The one place a class loader was produced from the caller in getBundleImpl it > was rewritten to route through the getCallerModule method: > > final ClassLoader loader = (caller != null) ? > caller.getClassLoader() : getLoader(getCallerModule(caller)); > > A JNI test was added which calls getBundle to fetch a test bundle from a > location added to the classpath, fetches a string out of the bundle and > verifies it, and calls clearCache. > > The javadoc was updated for the caller sensitive methods changed. Tim Prinzing has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision: - Merge branch 'master' into JDK-8280902 - remove unnecessary variable - more suggested changes - suggested changes - Update src/java.base/share/classes/java/util/ResourceBundle.java Co-authored-by: Mandy Chung - Use the unnamed module defined to the system class loader instead of the boot class loader. - JDK-8280902 ResourceBundle::getBundle may throw NPE when invoked by JNI code with no caller frame - Changes: - all: https://git.openjdk.java.net/jdk/pull/7663/files - new: https://git.openjdk.java.net/jdk/pull/7663/files/45f9b37b..6600e1f5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7663=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7663=03-04 Stats: 17900 lines in 516 files changed: 12667 ins; 3251 del; 1982 mod Patch: https://git.openjdk.java.net/jdk/pull/7663.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7663/head:pull/7663 PR: https://git.openjdk.java.net/jdk/pull/7663
Re: RFR: JDK-8280902 ResourceBundle::getBundle may throw NPE when invoked by JNI code with no caller frame [v4]
On Thu, 3 Mar 2022 16:55:46 GMT, Tim Prinzing wrote: >> The caller class returned by Reflection::getCallerClass was used to gain >> access to it's module in most cases and class loader in one case. I added a >> method to translate the caller class to caller module so that the decision >> of what module represents the caller with no stack frame is made in a single >> place. Calls made to caller.getModule() were replaced with >> getCallerModule(caller) which returns the system class loader unnamed module >> if the caller is null. >> >> The one place a class loader was produced from the caller in getBundleImpl >> it was rewritten to route through the getCallerModule method: >> >> final ClassLoader loader = (caller != null) ? >> caller.getClassLoader() : getLoader(getCallerModule(caller)); >> >> A JNI test was added which calls getBundle to fetch a test bundle from a >> location added to the classpath, fetches a string out of the bundle and >> verifies it, and calls clearCache. >> >> The javadoc was updated for the caller sensitive methods changed. > > Tim Prinzing has updated the pull request incrementally with one additional > commit since the last revision: > > more suggested changes Build changes look good. Can't say anything about the rest. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7663
Re: RFR: JDK-8280902 ResourceBundle::getBundle may throw NPE when invoked by JNI code with no caller frame [v4]
On Thu, 3 Mar 2022 16:55:46 GMT, Tim Prinzing wrote: >> The caller class returned by Reflection::getCallerClass was used to gain >> access to it's module in most cases and class loader in one case. I added a >> method to translate the caller class to caller module so that the decision >> of what module represents the caller with no stack frame is made in a single >> place. Calls made to caller.getModule() were replaced with >> getCallerModule(caller) which returns the system class loader unnamed module >> if the caller is null. >> >> The one place a class loader was produced from the caller in getBundleImpl >> it was rewritten to route through the getCallerModule method: >> >> final ClassLoader loader = (caller != null) ? >> caller.getClassLoader() : getLoader(getCallerModule(caller)); >> >> A JNI test was added which calls getBundle to fetch a test bundle from a >> location added to the classpath, fetches a string out of the bundle and >> verifies it, and calls clearCache. >> >> The javadoc was updated for the caller sensitive methods changed. > > Tim Prinzing has updated the pull request incrementally with one additional > commit since the last revision: > > more suggested changes Marked as reviewed by mchung (Reviewer). src/java.base/share/classes/java/util/ResourceBundle.java line 1570: > 1568: Module callerModule = (caller != null) ? caller.getModule() > 1569: : ClassLoader.getSystemClassLoader().getUnnamedModule(); > 1570: return callerModule; nit: `callerModule` variable is not needed. It can simply do: return (caller != null) ? caller.getModule() : ClassLoader.getSystemClassLoader().getUnnamedModule(); - PR: https://git.openjdk.java.net/jdk/pull/7663
Re: RFR: JDK-8280902 ResourceBundle::getBundle may throw NPE when invoked by JNI code with no caller frame [v4]
> The caller class returned by Reflection::getCallerClass was used to gain > access to it's module in most cases and class loader in one case. I added a > method to translate the caller class to caller module so that the decision of > what module represents the caller with no stack frame is made in a single > place. Calls made to caller.getModule() were replaced with > getCallerModule(caller) which returns the system class loader unnamed module > if the caller is null. > > The one place a class loader was produced from the caller in getBundleImpl it > was rewritten to route through the getCallerModule method: > > final ClassLoader loader = (caller != null) ? > caller.getClassLoader() : getLoader(getCallerModule(caller)); > > A JNI test was added which calls getBundle to fetch a test bundle from a > location added to the classpath, fetches a string out of the bundle and > verifies it, and calls clearCache. > > The javadoc was updated for the caller sensitive methods changed. Tim Prinzing has updated the pull request incrementally with one additional commit since the last revision: more suggested changes - Changes: - all: https://git.openjdk.java.net/jdk/pull/7663/files - new: https://git.openjdk.java.net/jdk/pull/7663/files/eeb2d0fa..45f9b37b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7663=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7663=02-03 Stats: 31 lines in 2 files changed: 2 ins; 13 del; 16 mod Patch: https://git.openjdk.java.net/jdk/pull/7663.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7663/head:pull/7663 PR: https://git.openjdk.java.net/jdk/pull/7663
Re: RFR: JDK-8280902 ResourceBundle::getBundle may throw NPE when invoked by JNI code with no caller frame [v3]
On Wed, 2 Mar 2022 22:21:03 GMT, Mandy Chung wrote: >> Tim Prinzing has updated the pull request incrementally with one additional >> commit since the last revision: >> >> suggested changes > > test/jdk/java/util/ResourceBundle/exeNullCallerResourceBundle/exeNullCallerResourceBundle.c > line 28: > >> 26: >> 27: #include "jni.h" >> 28: #undef NDEBUG > > is this line unintended? Forcing the assert() seems like a good idea in a test - PR: https://git.openjdk.java.net/jdk/pull/7663
Re: RFR: JDK-8280902 ResourceBundle::getBundle may throw NPE when invoked by JNI code with no caller frame [v3]
On Wed, 2 Mar 2022 21:53:01 GMT, Tim Prinzing wrote:
>> The caller class returned by Reflection::getCallerClass was used to gain
>> access to it's module in most cases and class loader in one case. I added a
>> method to translate the caller class to caller module so that the decision
>> of what module represents the caller with no stack frame is made in a single
>> place. Calls made to caller.getModule() were replaced with
>> getCallerModule(caller) which returns the system class loader unnamed module
>> if the caller is null.
>>
>> The one place a class loader was produced from the caller in getBundleImpl
>> it was rewritten to route through the getCallerModule method:
>>
>> final ClassLoader loader = (caller != null) ?
>> caller.getClassLoader() : getLoader(getCallerModule(caller));
>>
>> A JNI test was added which calls getBundle to fetch a test bundle from a
>> location added to the classpath, fetches a string out of the bundle and
>> verifies it, and calls clearCache.
>>
>> The javadoc was updated for the caller sensitive methods changed.
>
> Tim Prinzing has updated the pull request incrementally with one additional
> commit since the last revision:
>
> suggested changes
Thanks for the update.
src/java.base/share/classes/java/util/ResourceBundle.java line 1567:
> 1565: * module will be used.
> 1566: * @param caller
> 1567: * @return
Maybe you intended to make this `/* */` instead of javadoc? no need to
have `@param` and `@return` for this simple method.
src/java.base/share/classes/java/util/ResourceBundle.java line 2251:
> 2249: @CallerSensitive
> 2250: public static final void clearCache() {
> 2251: final Module callerModule =
> getCallerModule(Reflection.getCallerClass());
nit: final can be dropped here.
test/jdk/java/util/ResourceBundle/exeNullCallerResourceBundle/NullCallerResourceBundle.java
line 52:
> 50: import java.nio.file.Paths;
> 51: import java.util.Properties;
> 52: import java.util.ResourceBundle;
nit: good to keep the imports in alphabetic order.
test/jdk/java/util/ResourceBundle/exeNullCallerResourceBundle/NullCallerResourceBundle.java
line 74:
> 72:
> 73: // set up shared library path
> 74: final String sharedLibraryPathEnvName =
> Platform.sharedLibraryPathVariableName();
Nit: `final` adds noise but no other value to the test. Can consider dropping
it.
test/jdk/java/util/ResourceBundle/exeNullCallerResourceBundle/NullCallerResourceBundle.java
line 89:
> 87:
> 88:
> 89: private static void makePropertiesFile() throws IOException {
This can be removed?
test/jdk/java/util/ResourceBundle/exeNullCallerResourceBundle/exeNullCallerResourceBundle.c
line 28:
> 26:
> 27: #include "jni.h"
> 28: #undef NDEBUG
is this line unintended?
-
PR: https://git.openjdk.java.net/jdk/pull/7663
Re: RFR: JDK-8280902 ResourceBundle::getBundle may throw NPE when invoked by JNI code with no caller frame [v3]
> The caller class returned by Reflection::getCallerClass was used to gain > access to it's module in most cases and class loader in one case. I added a > method to translate the caller class to caller module so that the decision of > what module represents the caller with no stack frame is made in a single > place. Calls made to caller.getModule() were replaced with > getCallerModule(caller) which returns the system class loader unnamed module > if the caller is null. > > The one place a class loader was produced from the caller in getBundleImpl it > was rewritten to route through the getCallerModule method: > > final ClassLoader loader = (caller != null) ? > caller.getClassLoader() : getLoader(getCallerModule(caller)); > > A JNI test was added which calls getBundle to fetch a test bundle from a > location added to the classpath, fetches a string out of the bundle and > verifies it, and calls clearCache. > > The javadoc was updated for the caller sensitive methods changed. Tim Prinzing has updated the pull request incrementally with one additional commit since the last revision: suggested changes - Changes: - all: https://git.openjdk.java.net/jdk/pull/7663/files - new: https://git.openjdk.java.net/jdk/pull/7663/files/9e8fb193..eeb2d0fa Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7663=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7663=01-02 Stats: 48 lines in 1 file changed: 6 ins; 41 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7663.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7663/head:pull/7663 PR: https://git.openjdk.java.net/jdk/pull/7663
Re: RFR: JDK-8280902 ResourceBundle::getBundle may throw NPE when invoked by JNI code with no caller frame [v2]
> The caller class returned by Reflection::getCallerClass was used to gain > access to it's module in most cases and class loader in one case. I added a > method to translate the caller class to caller module so that the decision of > what module represents the caller with no stack frame is made in a single > place. Calls made to caller.getModule() were replaced with > getCallerModule(caller) which returns the system class loader unnamed module > if the caller is null. > > The one place a class loader was produced from the caller in getBundleImpl it > was rewritten to route through the getCallerModule method: > > final ClassLoader loader = (caller != null) ? > caller.getClassLoader() : getLoader(getCallerModule(caller)); > > A JNI test was added which calls getBundle to fetch a test bundle from a > location added to the classpath, fetches a string out of the bundle and > verifies it, and calls clearCache. > > The javadoc was updated for the caller sensitive methods changed. Tim Prinzing has updated the pull request incrementally with one additional commit since the last revision: Update src/java.base/share/classes/java/util/ResourceBundle.java Co-authored-by: Mandy Chung - Changes: - all: https://git.openjdk.java.net/jdk/pull/7663/files - new: https://git.openjdk.java.net/jdk/pull/7663/files/30778063..9e8fb193 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7663=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7663=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7663.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7663/head:pull/7663 PR: https://git.openjdk.java.net/jdk/pull/7663
Re: RFR: JDK-8280902 ResourceBundle::getBundle may throw NPE when invoked by JNI code with no caller frame
On Wed, 2 Mar 2022 18:56:40 GMT, Tim Prinzing wrote:
> The caller class returned by Reflection::getCallerClass was used to gain
> access to it's module in most cases and class loader in one case. I added a
> method to translate the caller class to caller module so that the decision of
> what module represents the caller with no stack frame is made in a single
> place. Calls made to caller.getModule() were replaced with
> getCallerModule(caller) which returns the system class loader unnamed module
> if the caller is null.
>
> The one place a class loader was produced from the caller in getBundleImpl it
> was rewritten to route through the getCallerModule method:
>
> final ClassLoader loader = (caller != null) ?
> caller.getClassLoader() : getLoader(getCallerModule(caller));
>
> A JNI test was added which calls getBundle to fetch a test bundle from a
> location added to the classpath, fetches a string out of the bundle and
> verifies it, and calls clearCache.
>
> The javadoc was updated for the caller sensitive methods changed.
Instead of sprinkling the null caller text in the javadoc in each `getBundle`
method, we can specify that that in the class specification, for example, after
the "Resource bundles in other modules and class path" section.
In cases where the {@code getBundle} factory method is called from a context
where there is no caller frame on the stack (e.g. when called directly from
a JNI attached thread), the caller module is default to the unnamed module for
the
{@linkplain ClassLoader#getSystemClassLoader system class loader}.
What do you think?
src/java.base/share/classes/java/util/ResourceBundle.java line 1588:
> 1586: Control control) {
> 1587: final ClassLoader loader = (caller != null) ?
> 1588: caller.getClassLoader() :
> getLoader(getCallerModule(caller));
Suggestion:
ClassLoader loader = getLoader(getCallerModule(caller));
This can be simplified as above. I would drop `final` too as it only adds
noise to the code.
-
PR: https://git.openjdk.java.net/jdk/pull/7663
Re: RFR: JDK-8280902 ResourceBundle::getBundle may throw NPE when invoked by JNI code with no caller frame
On Wed, 2 Mar 2022 18:56:40 GMT, Tim Prinzing wrote: > The caller class returned by Reflection::getCallerClass was used to gain > access to it's module in most cases and class loader in one case. I added a > method to translate the caller class to caller module so that the decision of > what module represents the caller with no stack frame is made in a single > place. Calls made to caller.getModule() were replaced with > getCallerModule(caller) which returns the system class loader unnamed module > if the caller is null. > > The one place a class loader was produced from the caller in getBundleImpl it > was rewritten to route through the getCallerModule method: > > final ClassLoader loader = (caller != null) ? > caller.getClassLoader() : getLoader(getCallerModule(caller)); > > A JNI test was added which calls getBundle to fetch a test bundle from a > location added to the classpath, fetches a string out of the bundle and > verifies it, and calls clearCache. > > The javadoc was updated for the caller sensitive methods changed. Looks good to me. I believe a CSR is needed for this change. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7663
RFR: JDK-8280902 ResourceBundle::getBundle may throw NPE when invoked by JNI code with no caller frame
The caller class returned by Reflection::getCallerClass was used to gain access to it's module in most cases and class loader in one case. I added a method to translate the caller class to caller module so that the decision of what module represents the caller with no stack frame is made in a single place. Calls made to caller.getModule() were replaced with getCallerModule(caller) which returns the system class loader unnamed module if the caller is null. The one place a class loader was produced from the caller in getBundleImpl it was rewritten to route through the getCallerModule method: final ClassLoader loader = (caller != null) ? caller.getClassLoader() : getLoader(getCallerModule(caller)); A JNI test was added which calls getBundle to fetch a test bundle from a location added to the classpath, fetches a string out of the bundle and verifies it, and calls clearCache. The javadoc was updated for the caller sensitive methods changed. - Commit messages: - Use the unnamed module defined to the system class loader instead of the - JDK-8280902 ResourceBundle::getBundle may throw NPE when invoked by JNI code with no caller frame Changes: https://git.openjdk.java.net/jdk/pull/7663/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7663=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8280902 Stats: 265 lines in 4 files changed: 254 ins; 3 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/7663.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7663/head:pull/7663 PR: https://git.openjdk.java.net/jdk/pull/7663
