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 <uschind...@apache.org> wrote: > > Hi, > > Alan Bateman [mailto:alan.bate...@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 >
