Method to unmap MappedByteBuffer

[Luke Hutchison]

(specific questions are bolded below)

(For context, I have read this bug report, explaining why DirectByteBuffer
doesn't yet have unmap support:
https://bugs.openjdk.java.net/browse/JDK-4724038 )

Before JDK 9, there were only two mechanisms for unapping a MappedByteBuffer
:

(1) dropping all references and then waiting for GC (or calling System.gc()
); or
(2) calling ((DirectBuffer) buf).cleaner().clean() .

On (1): Having to call System.gc() to unmap a DirectByteBuffer is not a
good general solution, because (a) GC pause time can be excessive, and (b)
System.gc() is not even guaranteed to trigger GC.

On (2): In JDK 9+, this method gives an ugly (and unsuppressable) "An
illegal reflective access operation has occurred" warning on stderr. You
can instead call sun.misc.Unsafe.invokeCleaner(buf) , and that makes the
same call shown above in (2), without the warning. But it appears that
sun.misc.Unsafe is going away. Most of the calls in that class have been
directly proxied to jdk.internal.misc.Unsafe , with one exception: the
invokeCleaner(ByteBuffer) method is not proxied across to
jdk.internal.misc.Unsafe , and there is no corresponding method
jdk.internal.misc.Unsafe::invokeCleaner . *Was that an intentional or
unintentional omission?*

Right now solution (2) necessitates a "requires jdk.unsupported" line in
the module descriptor. *What are the ramifications of depending upon
jdk.unsupported*, other than that your code may not work on future JDK
versions? For example, I saw somewhere that callers of any library that
depends upon a dependency like jdk.unsupported may have to add
--add-modules=ALL-DEFAULT to their runtime to work?

There needs to be a supported way of doing unmap in Java. Having mmap
support without corresponding unmap support is like not supporting a close()
method in InputStream or OutputStream, and having to drop references to the
stream and then wait for GC for streams to be closed (which would be quite
ridiculous, and very IO-unsafe with cached filesystems).

This is especially an issue because of the 64k limit to the number of
concurrent mmap'd files in Linux, and the problem that you cannot delete a
file while it is mmap'd in Windows. Both these problems are hit regularly
in Java because of the lack of clean unmap support:

http://www.mapdb.org/blog/mmap_files_alloc_and_jvm_crash/

*Why not have DirectByteBuffer and/or MappedByteBuffer implement
AutoCloseable, and then have the close() method call the cleaner?*
(Discussion on security is further down this email.) If you don't care
about manually unmapping, or you care about security, you can just let GC
do its thing. If on the other hand you do care about unmapping the buffer
immediately, you can close it yourself, or use try-with-resources.

One of the problems with manually unmapping a DirectByteBuffer is that if
you use the buffer after it is cleaned, you get a VM crash with "A fatal
error has been detected by the Java Runtime Environment:
EXCEPTION_ACCESS_VIOLATION". With support for unmapping DirectByteBuffer
instances, this should be softened to an exception (the whole VM should not
crash!). But if the mechanism for closing DirectByteBuffer instances is
simply AutoCloseable, the chance of this error or exception getting
triggered will be minimized, since a common usecase for DirectByteBuffer
would be a try-with-resources block (or at least the user would be notified
by their IDE that they should close() the buffer when finished).

On the security issue that Mark Reinhold brought up in his 2005 bug report,
to do with leaking access to another user's mapped file contents if a
subsequent buffer is mapped into the same address space: *why not just have
a field in the DirectByteBuffer that indicates whether the buffer has been
closed?* That requires one field value check per API call, which would slow
down the use of DirectByteBuffer a bit (or maybe a lot, for code that makes
a lot of one-byte reads from the DirectByteBuffer) -- but it would make the
API 100% safe, and frankly safety should never be sacrificed for
performance. You could even do this in the native code underpinning the
DirectByteBuffer for speed.

Another possibility would be to swap out the class' definition, when the
buffer is unmapped, for another definition that simply throws an unchecked
exception from every method (that way, the test to see if the buffer has
been closed will not have to be performed). *Why not have some native code
in the MappedByteBuffer::close method that overwrites the class pointer for
the object with another class definition of the same supertype
(MappedByteBuffer) whose methods simply throw an unchecked exception?* It's
a bit funky that an object's concrete class would suddenly change, and that
could trip up some code that uses the object's class as a map key or
similar, but at least future calls to the object's methods will throw an
exception, and frankly there is enough that is broken with mapped byte
buffers right now that making the situation "less broken" would be a huge
improvement.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20190107/e44da466/attachment.html>