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

[Daniel Fuchs]

Hi Naoto,

On 14/01/17 00:54, Naoto Sato wrote:
diff -r a6d3c80ea436
src/java.base/share/classes/java/util/ResourceBundle.java
--- a/src/java.base/share/classes/java/util/ResourceBundle.java
+++ b/src/java.base/share/classes/java/util/ResourceBundle.java
@@ -2192,7 +2192,7 @@
     private static void clearCacheImpl(Module callerModule, ClassLoader
loader) {
         cacheList.keySet().removeIf(
             key -> key.getCallerModule() == callerModule &&
-                   getLoader(key.getModule()) == loader
+                   key.getModule() != null &&
getLoader(key.getModule()) == loader
         );
     }
This is the only occurence where CacheKey's modules are passed to
getLoader(Module). Entire webrev is here:

http://cr.openjdk.java.net/~naoto/8171139/webrev.05/

I think you need to introduce a new static method here, because
there's no guarantee that a module won't be GC'ed between
two calls to key.getModule() - the first call could return
non null and the second could return null:

private static boolean isMatching(CacheKey key, Module callerModule, 
ClassLoader loader) {
    final Module module = key.getModule();
    return key.getCallerModule() == callerModule &&
           module != null && getLoader(module) == loader;
}

and then use that in the lambda.

A related question is whether a CacheKey whose module is null
is stale? If so maybe clearCacheImpl could also be changed to
removed stale keys as well as matching keys?

private static boolean isMatching(CacheKey key, Module callerModule, 
ClassLoader loader) {
    final Module km = key.getModule();
    final Module kcm = key.getCallerModule();
    return kcm == null || km == null || kcm == callerModule &&
           getLoader(km) == loader;
}


best regards,

-- daniel