What I wanted to say is that you are creating a new CacheKey which is used to
load a resource bundle and if found, it will be put in the cacheList.
It seems wrong to continue the request for loading a bundle but its caller
module and target module is GC’ed and put a NONEXISTENT_BUNDLE in the cache. I
consider it as an error state if we ever get there.
Mandy
> On Jan 13, 2017, at 4:54 PM, Naoto Sato <naoto.s...@oracle.com> wrote:
>
> Modified as:
>
> 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/
>
> Naoto
>
> On 01/13/2017 04:25 PM, Mandy Chung wrote:
>>
>>> On Jan 13, 2017, at 4:05 PM, Naoto Sato <naoto.s...@oracle.com> wrote:
>>>>
>>>> Daniel asks whether there is any possibility that the target module or
>>>> caller module would be GC’ed.
>>>>
>>>> CacheKey is copied in two places, one in findBundle and the other is
>>>> putBundleInCache which is actually passed with a newly cloned CacheKey.
>>>> IIUC, the cacheKey passed to findBundle ia always a new instance (as it
>>>> subsequently sets to different locale during the lookup). Its caller
>>>> module is the caller on the stack and the target module is also a strong
>>>> reference either caller’s module, unnamed module from a given class
>>>> loader, or a given module.
>>>>
>>>> Naoto will have to double check.
>>>
>>> I think this is correct. The current way of using CacheKey is safe from its
>>> modules to be GC'ed. However in general, it'd be good prepare them to be
>>> GC'ed. I modified the impl to hold them in local variables preventing them
>>> to be GC'ed before instantiating new References. (Also I modified not to
>>> call the other constructor in the copy constructor, reinstating some piece
>>> what Peter's diff originally had). So here is the updated webrev:
>>>
>>> http://cr.openjdk.java.net/~naoto/8171139/webrev.04/
>>
>> So callerRef and moduleRef may be null. getLoader(null) may be called which
>> will throw NPE.
>>
>> Mandy
>>
>
