RFR (Unraised): JDK8 ResourceBundle vulnerable to GC

[Adam Farley8]

> Alan Bateman <Alan.Bateman at oracle.com> wrote on 16/07/2018 15:00:16:

> From: Alan Bateman <Alan.Bateman at oracle.com>
> To: Peter Levart <peter.levart at gmail.com>, Adam Farley8 
> <adam.farley at uk.ibm.com>, core-libs-dev <core-libs-dev at openjdk.java.net>
> Date: 16/07/2018 14:59
> Subject: Re: RFR (Unraised): JDK8 ResourceBundle vulnerable to GC
> 
> On 16/07/2018 14:12, Peter Levart wrote:
> >
> > Checking the differences shows that instead of 
> > WeakReference<ClassLoader>, CacheKey uses two WeakReference<Module> 
> > instead in JDK 9+. One is the caller module and the other is the 
> > module of the bundle (unnamed module if ClassLoader is used to search 
> > for the bundle). The caller module is guaranteed to stay reachable for 

> > the entire call (as it holds the calling code), but the Module of the 
> > bundle could go out of scope during the call, so I think there is a 
> > theoretical possibility that the lookup CacheKey becomes non-equal to 
> > a key in the cache before the cache lookup is executed.
> This would require module layers or custom class loaders in the picture 
> for that to happen but I agree we should fix it.
> 
> -Alan
> 

So the JDK8 fix could be introduced as proposed (==). As for the JDK9+
issue, I believe that has already been solved. See the line at the end of
the Module-Module-String-Locale-Control getBundleImpl, where it says:

Reference.reachabilityFence(module);

As for JDK8, to confirm, are you agreeing that the correct fix for this
is the originally proposed == (where we compare the Class Loader with 
a blank static volatile Object (defined outside the method scope) at 
the end of the getBundleImpl class)?

So maybe:

-----------------------------------------
+1322
    /**
     * volatile reference object to guard the ClassLoader object
     * being garbage collected before getBundleImpl() method completes
     * the caching and retrieving of requested Resourcebundle object
     */
    private static volatile Object vo = new Object();


+1400
//Should never be true. Using the loader here prevents it being GC'd.
            if (loader == vo) throw new Error("Unexpected error.");
-----------------------------------------

Except in a proper webrev.

- Adam
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU