Hi Mandy, Hans,
Tried out the below (after removing my previous fix, the volatile
compare), and it appears to solve the problem just fine.
If I generate a webrev and get it attached to the bug, would one of you
mind approving the change?
----------...at the end of getBundleImpl in jdk8 ------------
keepAlive(loader);
return bundle;
}
//To keep the argument ClassLoader alive.
private static void keepAlive(ClassLoader loaderone){
//Do nothing.
}
----------------------
Best Regards
Adam Farley
Hans Boehm <hbo...@google.com> wrote on 15/08/2018 06:44:08:
> From: Hans Boehm <hbo...@google.com>
> To: Mandy Chung <mandy.ch...@oracle.com>
> Cc: adam.far...@uk.ibm.com, core-libs-dev <core-libs-
> d...@openjdk.java.net>, i18n-dev@openjdk.java.net
> Date: 15/08/2018 06:44
> Subject: Re: RFR 8209184: JDK8 ResourceBundle vulnerable to GC (fix
included)
>
> FWIW, since there was agreement that empty static methods should
> work in this context, that seems like the best option to me.
>
> On Tue, Aug 14, 2018 at 4:54 PM mandy chung <mandy.ch...@oracle.com>
wrote:
> Thanks for the information. Not sure what's the best option we can do
> in 8u. I think it's acceptable to have a fix that works in the
> current context (like empty static method).
>
> Thoughts?
>
> Mandy
>
> On 8/14/18 4:22 PM, Hans Boehm wrote:
> > I haven't looked at the details here, but comparing against a volatile
> > is not in general a portable way to keep an object live. Like empty
> > static methods, it may be fine in the current (OpenJDK) context.
> >
> > Volatile loads have roach motel semantics and can be advanced past
other
> > operations. The comparison can be done early, too, keeping only the
> > condition code. There is no guarantee the reference will still be
around
> > when you expect it.
> >
> > I haven't come up with a great compiler-independent replacement for
> > reachabilityFence.
> >
> > On Tue, Aug 14, 2018 at 8:25 AM mandy chung <mandy.ch...@oracle.com
> > <mailto:mandy.ch...@oracle.com>> wrote:
> >
> > Hi Adam,
> >
> > Have you tried Peter's suggestion if an empty static method taking
an
> > Object parameter? Does it work for you?
> >
> > Your proposed approach seems fine and I would suggest to put the
> > check in a static keepAlive method that will make it explicitly.
> >
> > Mandy
> >
> > On 8/10/18 8:42 AM, Adam Farley8 wrote:
> > > Hi All,
> > >
> > > This bug could be fixed by comparing the Class Loader with a
blank
> > > static volatile Object (defined outside the method scope) at
the
> > > end of the getBundleImpl class.
> > >
> > > E.g.
> > >
> > > -----------------------------------------
> > > +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.");
> > > -----------------------------------------
> > >
> > > Will upload a webrev after debate.
> > >
> > > - 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
> > >
> >
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
