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

[Uwe Schindler]

Hi all, hi Chris,

Thanks for the catch up! As we are continuously testing EA builds of OpenJDK with Apache Lucene, we will see the issues caused by this commit asap. So I would maybe delay our testing a bit, until the proposed changes of Alan are also committed and part of EA builds. Is there already an issue about that?

I opened an issue in the Apache Lucene tracker:
https://issues.apache.org/jira/browse/LUCENE-6989

I hope, we will find a good solution!

I had some other thoughts about the issue: Basically the forceful unmapping is only needed because the Grabage Collector never ever tries to finalize and GC the very small MappedByteBuffer instances, especially if they were living for longer time. But they are sitting on heavy resources, so maybe a solution to solve this would be:

- Add some special annotation in the JDK that is respected by all Garbage Collectors and makes them prefer those "heavy" instances. Something like @jdk.internal.Heavy
- Maybe add some close() method to MappedByteBuffer, but just make it a candidate for earlier Grabage Collection instead of doing risky unmapping. A solution may be to add it to some special GC queue or whatever. I have no idea if that’s possible, just tell GC: "I am done with this object, discard it asap if its unreachable!"

Uwe

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


> -----Original Message-----
> From: Chris Hegarty [mailto:chris.hegarty at oracle.com]
> Sent: Saturday, January 23, 2016 11:52 PM
> To: Uwe Schindler <uschindler at apache.org>
> Cc: 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
> 
> Just catching up…
> 
> I see no objection to the proposed patch, but the obvious concern ( that
> is highlighted in JEP 260 ), about the digging into the private internal
> details of NIO buffers.
> 
> I think Alan summarized the issues around providing a public API for
> releasing/unmapping native memory very well, and if this does not
> make it for JDK 9, then Alan’s alternative ( implements Runnable ) may
> be a better interim solution than the current use of reflection.
> 
> -Chris.
> 
> > On 23 Jan 2016, at 21:57, Uwe Schindler <uschindler at apache.org> wrote:
> >
> > Hi,
> >
> > Alan Bateman [mailto:Alan.Bateman at oracle.com] wrote:
> >> On 23/01/2016 19:23, Uwe Schindler wrote:
> >>> :
> >>>
> >>> What is the replacement for the following code (see marked
> BufferCleaner
> >> impl, which is our abstraction): https://goo.gl/DGYZZj
> >>>
> >>> The main reason why sun.misc.Cleaner was on the critical API list is NOT
> the
> >> case that somebody wanted to implement their own cleaner as
> replacement
> >> for finalization. The main reason (and this is *not* solved by the new APIs
> in
> >> java.lang.ref.Cleaner) is to *force* unmapping of direct and mmapped
> byte
> >> buffers.
> >> JEP 260 was updated recently to move this to an open issue and I think
> >> the text captures the issue correctly. Part of it that we can't have
> >> types in java.base depending on types in other modules and this is why
> >> Buffers can't have fields of type sun.misc.Cleaner or any type in sun.misc.
> >>
> >> As has come several times, there are huge security and reliability
> >> issues that arise when releasing or unmapping memory that is still
> >> accessible via reachable objects. Proposed solutions over the years have
> >> traded performance or were limited to platforms where we could
> >> atomically remap. JDK 9 may be the time to try yet again or else just
> >> introduce a JDK-specific API to unmap that comes with a warning in 96pt
> >> font. It might have to be disabled completely when running with a
> >> security manager. Alternatively the burden of the current hack could be
> >> reduced by having Cleaner implement Runnable with a run method that
> >> invokes clean. That would avoid needing to cast to a JDK-internal type
> >> and so avoiding needing to use -XaddExports to break encapsulation.
> >
> > We know this problem. Because of this, e.g. Elasticsearch runs with a
> security manager and only allows lucene-core.jar access to do this reflection
> on sun.misc package (using the correct permission, only given to JAR file).
> >
> > I was about to suggest the same thing: Maybe add a public API to byte
> buffer, that is documented to throw SecurityException. Add a new
> Runtime/WhateverPermission that can be enabled to work around that (e.g.,
> Elasticsearch would do this and allow this Permission only to lucene-core.jar).
> In addition add red and blinking warnings to this method :-)
> >
> > In any case, a possible solution was proposed by Andrew Haley (he had
> some ideas in the past, I think we brought this up the last time a few months
> ago) with a small slowdown, but still providing better ByteBuffer
> performance than in Java 7 or Java 8.
> >
> > I would hope for a solution that installs a SIGSEGV signal handler in the JDK,
> that translates any completely illegal access that would otherwise lead to a
> crash of the JDK to some Java Exception. The security issues you also
> mention do not apply for us (e.g., reading memory from another web
> application in the same JVM), so a new permission for SecurityManager
> would also help here.
> >
> > Uwe
> >