RFR [9] 8148117: Move sun.misc.Cleaner to jdk.internal.ref

[Uwe Schindler]

Hi,

looks good to me. Once we have EA builds with that I will adapt the ByteBufferIndexInput code in Lucene.

One thing about the Runable: This should work perfectly fine, because we only need to make call the getCleaner() method accessible, call it and finally check if return type implements Runable (if not we fall back to pre Java 9 code). We can then cast and invoke the cleaner without any further reflection.

But there is now some security related issue: The reflection on DirectBuffer should work in most environments (also with security managers), so you can get the Cleaner instance with a doPrivileged (ideally). But as this cleaner instance was previously inside sun.misc package, and you needed RuntimePermission "accessInPackage.sun.misc" to call and make the clean method accessible. But with the new patch you no longer need this permission, making it easier to invoke that method.

In my opinion, you should add some other runtime permission for safety, which is checked on calling the run() method. This would make it safe, because as soon as you have SecurityManager you would need to give explicit permission to the code.

Uwe

-----
Uwe Schindler
uschindler at apache.org 
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/


> -----Original Message-----
> From: core-libs-dev [mailto:core-libs-dev-bounces at openjdk.java.net] On
> Behalf Of Chris Hegarty
> Sent: Tuesday, January 26, 2016 2:56 PM
> To: Alan Bateman <Alan.Bateman at oracle.com>; core-libs-dev <core-libs-
> dev at openjdk.java.net>
> Subject: Re: RFR [9] 8148117: Move sun.misc.Cleaner to jdk.internal.ref
> 
> It is wonderful to see the various ideas on this thread about the longer
> term solution to the prompt releasing of direct buffer native memory. I
> do not want to obstruct that ( it is very informative ), but I’d like to warp up
> the review on the actual moving of Cleaner. To that end, I’ve update the
> webrev as per Alan’s comments and suggestion ( to extend Runnable ).
> 
> http://cr.openjdk.java.net/~chegar/8148117/
> 
> -Chris.
> 
> On 23 Jan 2016, at 16:36, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> 
> > On 23/01/2016 16:16, Chris Hegarty wrote:
> >> :
> >>
> >> Webrev:
> >>   http://cr.openjdk.java.net/~chegar/8148117/
> >>
> >>
> > This has to move to your patch looks okay. You might need to update the
> TEST.groups to ensure that the existOnThrow tests will be run by jdk_core.
> >
> > -Alan.