6816049: (bf) MappedByteBuffer.force() method does not flush data correctly
Reviewed-by: chegar
--- a/jdk/src/share/classes/java/nio/Direct-X-Buffer.java.template Fri Oct 22 09:20:09 2010 +0100
+++ b/jdk/src/share/classes/java/nio/Direct-X-Buffer.java.template Fri Oct 22 17:40:31 2010 +0100
@@ -27,6 +27,7 @@
package java.nio;
+import java.io.FileDescriptor;
import sun.misc.Cleaner;
import sun.misc.Unsafe;
import sun.misc.VM;
@@ -114,7 +115,7 @@
//
Direct$Type$Buffer$RW$(int cap) { // package-private
#if[rw]
- super(-1, 0, cap, cap, false);
+ super(-1, 0, cap, cap);
boolean pa = VM.isDirectMemoryPageAligned();
int ps = Bits.pageSize();
long size = Math.max(1L, (long)cap + (pa ? ps : 0));
@@ -145,7 +146,7 @@
// Invoked only by JNI: NewDirectByteBuffer(void*, long)
//
private Direct$Type$Buffer(long addr, int cap) {
- super(-1, 0, cap, cap, false);
+ super(-1, 0, cap, cap);
address = addr;
cleaner = null;
}
@@ -154,14 +155,17 @@
// For memory-mapped buffers -- invoked by FileChannelImpl via reflection
//
- protected Direct$Type$Buffer$RW$(int cap, long addr, Runnable unmapper) {
+ protected Direct$Type$Buffer$RW$(int cap, long addr,
+ FileDescriptor fd,
+ Runnable unmapper)
+ {
#if[rw]
- super(-1, 0, cap, cap, true);
+ super(-1, 0, cap, cap, fd);
address = addr;
viewedBuffer = null;
cleaner = Cleaner.create(this, unmapper);
#else[rw]
- super(cap, addr, unmapper);
+ super(cap, addr, fd, unmapper);
#end[rw]
}
--- a/jdk/src/share/classes/java/nio/MappedByteBuffer.java Fri Oct 22 09:20:09 2010 +0100
+++ b/jdk/src/share/classes/java/nio/MappedByteBuffer.java Fri Oct 22 17:40:31 2010 +0100
@@ -25,6 +25,7 @@
package java.nio;
+import java.io.FileDescriptor;
import sun.misc.Unsafe;
@@ -71,26 +72,26 @@
// for optimization purposes, it's easier to do it the other way around.
// This works because DirectByteBuffer is a package-private class.
- // Volatile to make sure that the finalization thread sees the current
- // value of this so that a region is not accidentally unmapped again later.
- volatile boolean isAMappedBuffer; // package-private
+ // For mapped buffers, a FileDescriptor that may be used for mapping
+ // operations if valid; null if the buffer is not mapped.
+ private final FileDescriptor fd;
// This should only be invoked by the DirectByteBuffer constructors
//
MappedByteBuffer(int mark, int pos, int lim, int cap, // package-private
- boolean mapped)
+ FileDescriptor fd)
{
super(mark, pos, lim, cap);
- isAMappedBuffer = mapped;
+ this.fd = fd;
}
MappedByteBuffer(int mark, int pos, int lim, int cap) { // package-private
super(mark, pos, lim, cap);
- isAMappedBuffer = false;
+ this.fd = null;
}
private void checkMapped() {
- if (!isAMappedBuffer)
+ if (fd == null)
// Can only happen if a luser explicitly casts a direct byte buffer
throw new UnsupportedOperationException();
}
@@ -191,13 +192,12 @@
checkMapped();
if ((address != 0) && (capacity() != 0)) {
long offset = mappingOffset();
- force0(mappingAddress(offset), mappingLength(offset));
+ force0(fd, mappingAddress(offset), mappingLength(offset));
}
return this;
}
private native boolean isLoaded0(long address, long length, int pageCount);
private native void load0(long address, long length);
- private native void force0(long address, long length);
-
+ private native void force0(FileDescriptor fd, long address, long length);
}
--- a/jdk/src/share/classes/sun/nio/ch/FileChannelImpl.java Fri Oct 22 09:20:09 2010 +0100
+++ b/jdk/src/share/classes/sun/nio/ch/FileChannelImpl.java Fri Oct 22 17:40:31 2010 +0100
@@ -699,15 +699,19 @@
static volatile long totalSize;
static volatile long totalCapacity;
- private long address;
- private long size;
- private int cap;
+ private volatile long address;
+ private final long size;
+ private final int cap;
+ private final FileDescriptor fd;
- private Unmapper(long address, long size, int cap) {
+ private Unmapper(long address, long size, int cap,
+ FileDescriptor fd)
+ {
assert (address != 0);
this.address = address;
this.size = size;
this.cap = cap;
+ this.fd = fd;
synchronized (Unmapper.class) {
count++;
@@ -722,6 +726,15 @@
unmap0(address, size);
address = 0;
+ // if this mapping has a valid file descriptor then we close it
+ if (fd.valid()) {
+ try {
+ nd.close(fd);
+ } catch (IOException ignore) {
+ // nothing we can do
+ }
+ }
+
synchronized (Unmapper.class) {
count--;
totalSize -= size;
@@ -784,10 +797,12 @@
}
if (size == 0) {
addr = 0;
+ // a valid file descriptor is not required
+ FileDescriptor dummy = new FileDescriptor();
if ((!writable) || (imode == MAP_RO))
- return Util.newMappedByteBufferR(0, 0, null);
+ return Util.newMappedByteBufferR(0, 0, dummy, null);
else
- return Util.newMappedByteBuffer(0, 0, null);
+ return Util.newMappedByteBuffer(0, 0, dummy, null);
}
int pagePosition = (int)(position % allocationGranularity);
@@ -813,14 +828,31 @@
}
}
+ // On Windows, and potentially other platforms, we need an open
+ // file descriptor for some mapping operations.
+ FileDescriptor mfd;
+ try {
+ mfd = nd.duplicateForMapping(fd);
+ } catch (IOException ioe) {
+ unmap0(addr, mapSize);
+ throw ioe;
+ }
+
assert (IOStatus.checkAll(addr));
assert (addr % allocationGranularity == 0);
int isize = (int)size;
- Unmapper um = new Unmapper(addr, size + pagePosition, isize);
- if ((!writable) || (imode == MAP_RO))
- return Util.newMappedByteBufferR(isize, addr + pagePosition, um);
- else
- return Util.newMappedByteBuffer(isize, addr + pagePosition, um);
+ Unmapper um = new Unmapper(addr, mapSize, isize, mfd);
+ if ((!writable) || (imode == MAP_RO)) {
+ return Util.newMappedByteBufferR(isize,
+ addr + pagePosition,
+ mfd,
+ um);
+ } else {
+ return Util.newMappedByteBuffer(isize,
+ addr + pagePosition,
+ mfd,
+ um);
+ }
} finally {
threads.remove(ti);
end(IOStatus.checkAll(addr));
--- a/jdk/src/share/classes/sun/nio/ch/FileDispatcher.java Fri Oct 22 09:20:09 2010 +0100
+++ b/jdk/src/share/classes/sun/nio/ch/FileDispatcher.java Fri Oct 22 17:40:31 2010 +0100
@@ -45,4 +45,12 @@
abstract void release(FileDescriptor fd, long pos, long size)
throws IOException;
+
+ /**
+ * Returns a dup of fd if a file descriptor is required for
+ * memory-mapping operations, otherwise returns an invalid
+ * FileDescriptor (meaning a newly allocated FileDescriptor)
+ */
+ abstract FileDescriptor duplicateForMapping(FileDescriptor fd)
+ throws IOException;
}
--- a/jdk/src/share/classes/sun/nio/ch/Util.java Fri Oct 22 09:20:09 2010 +0100
+++ b/jdk/src/share/classes/sun/nio/ch/Util.java Fri Oct 22 17:40:31 2010 +0100
@@ -28,6 +28,7 @@
import java.lang.ref.SoftReference;
import java.lang.reflect.*;
import java.io.IOException;
+import java.io.FileDescriptor;
import java.nio.ByteBuffer;
import java.nio.MappedByteBuffer;
import java.nio.channels.*;
@@ -364,6 +365,7 @@
Constructor ctor = cl.getDeclaredConstructor(
new Class[] { int.class,
long.class,
+ FileDescriptor.class,
Runnable.class });
ctor.setAccessible(true);
directByteBufferConstructor = ctor;
@@ -381,6 +383,7 @@
}
static MappedByteBuffer newMappedByteBuffer(int size, long addr,
+ FileDescriptor fd,
Runnable unmapper)
{
MappedByteBuffer dbb;
@@ -390,6 +393,7 @@
dbb = (MappedByteBuffer)directByteBufferConstructor.newInstance(
new Object[] { new Integer(size),
new Long(addr),
+ fd,
unmapper });
} catch (InstantiationException e) {
throw new InternalError();
@@ -411,6 +415,7 @@
Constructor ctor = cl.getDeclaredConstructor(
new Class[] { int.class,
long.class,
+ FileDescriptor.class,
Runnable.class });
ctor.setAccessible(true);
directByteBufferRConstructor = ctor;
@@ -428,6 +433,7 @@
}
static MappedByteBuffer newMappedByteBufferR(int size, long addr,
+ FileDescriptor fd,
Runnable unmapper)
{
MappedByteBuffer dbb;
@@ -437,6 +443,7 @@
dbb = (MappedByteBuffer)directByteBufferRConstructor.newInstance(
new Object[] { new Integer(size),
new Long(addr),
+ fd,
unmapper });
} catch (InstantiationException e) {
throw new InternalError();
--- a/jdk/src/solaris/classes/sun/nio/ch/FileDispatcherImpl.java Fri Oct 22 09:20:09 2010 +0100
+++ b/jdk/src/solaris/classes/sun/nio/ch/FileDispatcherImpl.java Fri Oct 22 17:40:31 2010 +0100
@@ -94,6 +94,12 @@
preClose0(fd);
}
+ FileDescriptor duplicateForMapping(FileDescriptor fd) {
+ // file descriptor not required for mapping operations; okay
+ // to return invalid file descriptor.
+ return new FileDescriptor();
+ }
+
// -- Native methods --
static native int read0(FileDescriptor fd, long address, int len)
--- a/jdk/src/solaris/native/java/nio/MappedByteBuffer.c Fri Oct 22 09:20:09 2010 +0100
+++ b/jdk/src/solaris/native/java/nio/MappedByteBuffer.c Fri Oct 22 17:40:31 2010 +0100
@@ -82,8 +82,8 @@
JNIEXPORT void JNICALL
-Java_java_nio_MappedByteBuffer_force0(JNIEnv *env, jobject obj, jlong address,
- jlong len)
+Java_java_nio_MappedByteBuffer_force0(JNIEnv *env, jobject obj, jobject fdo,
+ jlong address, jlong len)
{
void* a = (void *)jlong_to_ptr(address);
int result = msync(a, (size_t)len, MS_SYNC);
--- a/jdk/src/windows/classes/sun/nio/ch/FileDispatcherImpl.java Fri Oct 22 09:20:09 2010 +0100
+++ b/jdk/src/windows/classes/sun/nio/ch/FileDispatcherImpl.java Fri Oct 22 17:40:31 2010 +0100
@@ -26,10 +26,11 @@
package sun.nio.ch;
import java.io.*;
+import sun.misc.SharedSecrets;
+import sun.misc.JavaIOFileDescriptorAccess;
class FileDispatcherImpl extends FileDispatcher
{
-
static {
Util.load();
}
@@ -94,6 +95,16 @@
close0(fd);
}
+ FileDescriptor duplicateForMapping(FileDescriptor fd) throws IOException {
+ // on Windows we need to keep a handle to the file
+ JavaIOFileDescriptorAccess fdAccess =
+ SharedSecrets.getJavaIOFileDescriptorAccess();
+ FileDescriptor result = new FileDescriptor();
+ long handle = duplicateHandle(fdAccess.getHandle(fd));
+ fdAccess.setHandle(result, handle);
+ return result;
+ }
+
//-- Native methods
static native int read0(FileDescriptor fd, long address, int len)
@@ -132,4 +143,5 @@
static native void closeByHandle(long fd) throws IOException;
+ static native long duplicateHandle(long fd) throws IOException;
}
--- a/jdk/src/windows/native/java/nio/MappedByteBuffer.c Fri Oct 22 09:20:09 2010 +0100
+++ b/jdk/src/windows/native/java/nio/MappedByteBuffer.c Fri Oct 22 17:40:31 2010 +0100
@@ -51,11 +51,11 @@
}
JNIEXPORT void JNICALL
-Java_java_nio_MappedByteBuffer_force0(JNIEnv *env, jobject obj, jlong address,
- jlong len)
+Java_java_nio_MappedByteBuffer_force0(JNIEnv *env, jobject obj, jobject fdo,
+ jlong address, jlong len)
{
void *a = (void *) jlong_to_ptr(address);
- int result;
+ BOOL result;
int retry;
/*
@@ -71,6 +71,30 @@
retry++;
} while (retry < 3);
+ /**
+ * FlushViewOfFile only initiates the writing of dirty pages to disk
+ * so we have to call FlushFileBuffers to and ensure they are written.
+ */
+ if (result != 0) {
+ // by right, the jfieldID initialization should be in a static
+ // initializer but we do it here instead to avoiding needing to
+ // load nio.dll during startup.
+ static jfieldID handle_fdID;
+ HANDLE h;
+ if (handle_fdID == NULL) {
+ jclass clazz = (*env)->FindClass(env, "java/io/FileDescriptor");
+ if (clazz == NULL)
+ return; // exception thrown
+ handle_fdID = (*env)->GetFieldID(env, clazz, "handle", "J");
+ }
+ h = jlong_to_ptr((*env)->GetLongField(env, fdo, handle_fdID));
+ result = FlushFileBuffers(h);
+ if (result == 0 && GetLastError() == ERROR_ACCESS_DENIED) {
+ // read-only mapping
+ result = 1;
+ }
+ }
+
if (result == 0) {
JNU_ThrowIOExceptionWithLastError(env, "Flush failed");
}
--- a/jdk/src/windows/native/sun/nio/ch/FileDispatcherImpl.c Fri Oct 22 09:20:09 2010 +0100
+++ b/jdk/src/windows/native/sun/nio/ch/FileDispatcherImpl.c Fri Oct 22 17:40:31 2010 +0100
@@ -32,6 +32,7 @@
#include <io.h>
#include "nio.h"
#include "nio_util.h"
+#include "jlong.h"
/**************************************************************
@@ -441,3 +442,15 @@
{
closeFile(env, fd);
}
+
+JNIEXPORT jlong JNICALL
+Java_sun_nio_ch_FileDispatcherImpl_duplicateHandle(JNIEnv *env, jclass this, jlong hFile)
+{
+ HANDLE hProcess = GetCurrentProcess();
+ HANDLE hResult;
+ BOOL res = DuplicateHandle(hProcess, hFile, hProcess, &hResult, 0, FALSE,
+ DUPLICATE_SAME_ACCESS);
+ if (res == 0)
+ JNU_ThrowIOExceptionWithLastError(env, "DuplicateHandle failed");
+ return ptr_to_jlong(hResult);
+}
--- a/jdk/test/java/nio/MappedByteBuffer/Basic.java Fri Oct 22 09:20:09 2010 +0100
+++ b/jdk/test/java/nio/MappedByteBuffer/Basic.java Fri Oct 22 17:40:31 2010 +0100
@@ -24,7 +24,6 @@
/* @test
* @bug 4462336 6799037
* @summary Simple MappedByteBuffer tests
- * @run main/othervm Basic
*/
import java.io.*;
@@ -76,5 +75,10 @@
throw new RuntimeException("Incorrect isReadOnly");
fc.close();
raf.close();
+
+ // clean-up
+ mbb = null;
+ System.gc();
+ Thread.sleep(500);
}
}