[9] RFR: 8171139: Simplify ResourceBundle.CacheKey and ClassLoader may not be needed

[Peter Levart]

Hi Naoto,

Sorry for joining late for review. Thanks for taking this on. I think it 
is shaping well...

On 01/14/2017 01:54 AM, Naoto Sato wrote:
>
>
> http://cr.openjdk.java.net/~naoto/8171139/webrev.05/
>

Tho things...

1. As said, the "cloning" of CacheKey has no purpose in the following 
section of code (lines 1685...1709):

             CacheKey constKey = new CacheKey(cacheKey);
             trace("findBundle: %d %s %s formats: %s%n", index, 
candidateLocales, cacheKey, formats);
             try {
                 if (module.isNamed()) {
                     bundle = loadBundle(cacheKey, formats, control, 
module, callerModule);
                 } else {
                     bundle = loadBundle(cacheKey, formats, control, 
expiredBundle);
                 }
                 if (bundle != null) {
                     if (bundle.parent == null) {
                         bundle.setParent(parent);
                     }
                     bundle.locale = targetLocale;
                     bundle = putBundleInCache(cacheKey, bundle, control);
                     return bundle;
                 }

                 // Put NONEXISTENT_BUNDLE in the cache as a mark that 
there's no bundle
                 // instance for the locale.
                 putBundleInCache(cacheKey, NONEXISTENT_BUNDLE, control);
             } finally {
                 if (constKey.getCause() instanceof InterruptedException) {
                     Thread.currentThread().interrupt();
                 }
             }

The 'constKey' is copied of the 'cacheKey' and is not used anywhere 
except in finally block where it is asked for .getCause() which will 
always be null since copying of CacheKey never copies the cause...

2. We can make sure that CacheKey copy constructor always copies a 
CacheKey that has non-cleared moduleRef and callerRef. The original 
cacheKey is created in getBundleImpl() from nun-null module and 
callerModule.  getBundleImpl() then calls down to findModule() with this 
cacheKey. If we keep a reference to module and callerModule in 
getBundleImpl() alive until the end of this method, we guarantee that 
moduleRef and callerRef will not be cleared while using such cacheKey to 
construct copies of it.

Here's those two changes applied to your webrev.05 and also a race in 
clearCacheImpl() fixed (as reported by Daniel Fuchs):

http://cr.openjdk.java.net/~plevart/jdk9-dev/8170772_ResourceBundle.caching/webrev.06/

What do you think?

Regards, Peter