Thank you again, Daniel & Mandy.

On 1/13/17 1:25 PM, Mandy Chung wrote:

On Jan 12, 2017, at 1:20 PM, Naoto Sato <naoto.s...@oracle.com> wrote:

Realized moduleRef & callerRef would never be null with this new version. Thus 
modified the patch as:

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

CacheKey
   Nit: line 576 can be moved after 581 to group the volatile fields together

As in the comment line 574, the first 4 fields are the actual keys for lookup, so I'd keep them that way.


 619             this(src.getName(), src.getLocale(), src.getModule(), 
src.getCallerModule());

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/

Naoto

Reply via email to