6631352: File{OutputStream,Writer} should implement atomic append mode using FILE_APPEND_DATA (win)
Reviewed-by: alanb, iris
--- a/jdk/make/java/java/mapfile-vers Mon Mar 10 14:32:51 2008 -0700
+++ b/jdk/make/java/java/mapfile-vers Mon Mar 10 14:32:51 2008 -0700
@@ -85,7 +85,6 @@
Java_java_io_FileOutputStream_close0;
Java_java_io_FileOutputStream_initIDs;
Java_java_io_FileOutputStream_open;
- Java_java_io_FileOutputStream_openAppend;
Java_java_io_FileOutputStream_write;
Java_java_io_FileOutputStream_writeBytes;
Java_java_io_FileSystem_getFileSystem;
--- a/jdk/src/share/classes/java/io/FileOutputStream.java Mon Mar 10 14:32:51 2008 -0700
+++ b/jdk/src/share/classes/java/io/FileOutputStream.java Mon Mar 10 14:32:51 2008 -0700
@@ -58,8 +58,6 @@
private FileChannel channel= null;
- private boolean append = false;
-
private final Object closeLock = new Object();
private volatile boolean closed = false;
private static final ThreadLocal<Boolean> runningFinalize =
@@ -200,12 +198,7 @@
}
fd = new FileDescriptor();
fd.incrementAndGetUseCount();
- this.append = append;
- if (append) {
- openAppend(name);
- } else {
- open(name);
- }
+ open(name, append);
}
/**
@@ -250,16 +243,12 @@
}
/**
- * Opens a file, with the specified name, for writing.
+ * Opens a file, with the specified name, for overwriting or appending.
* @param name name of file to be opened
+ * @param append whether the file is to be opened in append mode
*/
- private native void open(String name) throws FileNotFoundException;
-
- /**
- * Opens a file, with the specified name, for appending.
- * @param name name of file to be opened
- */
- private native void openAppend(String name) throws FileNotFoundException;
+ private native void open(String name, boolean append)
+ throws FileNotFoundException;
/**
* Writes the specified byte to this file output stream. Implements
@@ -383,7 +372,7 @@
public FileChannel getChannel() {
synchronized (this) {
if (channel == null) {
- channel = FileChannelImpl.open(fd, false, true, this, append);
+ channel = FileChannelImpl.open(fd, false, true, this);
/*
* Increment fd's use count. Invoking the channel's close()
--- a/jdk/src/share/classes/sun/nio/ch/FileChannelImpl.java Mon Mar 10 14:32:51 2008 -0700
+++ b/jdk/src/share/classes/sun/nio/ch/FileChannelImpl.java Mon Mar 10 14:32:51 2008 -0700
@@ -52,39 +52,37 @@
{
// Used to make native read and write calls
- private static NativeDispatcher nd;
+ private static final NativeDispatcher nd;
// Memory allocation size for mapping buffers
- private static long allocationGranularity;
+ private static final long allocationGranularity;
// Cached field for MappedByteBuffer.isAMappedBuffer
- private static Field isAMappedBufferField;
+ private static final Field isAMappedBufferField;
// File descriptor
- private FileDescriptor fd;
+ private final FileDescriptor fd;
// File access mode (immutable)
- private boolean writable;
- private boolean readable;
- private boolean appending;
+ private final boolean writable;
+ private final boolean readable;
// Required to prevent finalization of creating stream (immutable)
- private Object parent;
+ private final Object parent;
// Thread-safe set of IDs of native threads, for signalling
- private NativeThreadSet threads = new NativeThreadSet(2);
+ private final NativeThreadSet threads = new NativeThreadSet(2);
// Lock for operations involving position and size
- private Object positionLock = new Object();
+ private final Object positionLock = new Object();
private FileChannelImpl(FileDescriptor fd, boolean readable,
- boolean writable, Object parent, boolean append)
+ boolean writable, Object parent)
{
this.fd = fd;
this.readable = readable;
this.writable = writable;
this.parent = parent;
- this.appending = append;
}
// Invoked by getChannel() methods
@@ -94,14 +92,7 @@
boolean readable, boolean writable,
Object parent)
{
- return new FileChannelImpl(fd, readable, writable, parent, false);
- }
-
- public static FileChannel open(FileDescriptor fd,
- boolean readable, boolean writable,
- Object parent, boolean append)
- {
- return new FileChannelImpl(fd, readable, writable, parent, append);
+ return new FileChannelImpl(fd, readable, writable, parent);
}
private void ensureOpen() throws IOException {
@@ -134,15 +125,7 @@
// superclass AbstractInterruptibleChannel, but the isOpen logic in
// that method will prevent this method from being reinvoked.
//
- if (parent instanceof FileInputStream)
- ((FileInputStream)parent).close();
- else if (parent instanceof FileOutputStream)
- ((FileOutputStream)parent).close();
- else if (parent instanceof RandomAccessFile)
- ((RandomAccessFile)parent).close();
- else
- assert false;
-
+ ((java.io.Closeable)parent).close();
} else {
nd.close(fd);
}
@@ -218,8 +201,6 @@
if (!isOpen())
return 0;
ti = threads.add();
- if (appending)
- position(size());
do {
n = IOUtil.write(fd, src, -1, nd, positionLock);
} while ((n == IOStatus.INTERRUPTED) && isOpen());
@@ -244,8 +225,6 @@
if (!isOpen())
return 0;
ti = threads.add();
- if (appending)
- position(size());
do {
n = IOUtil.write(fd, srcs, nd);
} while ((n == IOStatus.INTERRUPTED) && isOpen());
@@ -1051,7 +1030,7 @@
private FileKey fileKey;
FileLockReference(FileLock referent,
- ReferenceQueue queue,
+ ReferenceQueue<FileLock> queue,
FileKey key) {
super(referent, queue);
this.fileKey = key;
@@ -1073,7 +1052,7 @@
new ConcurrentHashMap<FileKey, ArrayList<FileLockReference>>();
// reference queue for cleared refs
- private static ReferenceQueue queue = new ReferenceQueue();
+ private static ReferenceQueue<FileLock> queue = new ReferenceQueue<FileLock>();
// the enclosing file channel
private FileChannelImpl fci;
--- a/jdk/src/share/native/java/io/io_util.c Mon Mar 10 14:32:51 2008 -0700
+++ b/jdk/src/share/native/java/io/io_util.c Mon Mar 10 14:32:51 2008 -0700
@@ -95,7 +95,7 @@
fd = GET_FD(this, fid);
if (fd == -1) {
JNU_ThrowIOException(env, "Stream Closed");
- return -1;
+ return -1;
}
nread = IO_Read(fd, buf, len);
--- a/jdk/src/solaris/native/java/io/FileOutputStream_md.c Mon Mar 10 14:32:51 2008 -0700
+++ b/jdk/src/solaris/native/java/io/FileOutputStream_md.c Mon Mar 10 14:32:51 2008 -0700
@@ -53,13 +53,10 @@
*/
JNIEXPORT void JNICALL
-Java_java_io_FileOutputStream_open(JNIEnv *env, jobject this, jstring path) {
- fileOpen(env, this, path, fos_fd, O_WRONLY | O_CREAT | O_TRUNC);
-}
-
-JNIEXPORT void JNICALL
-Java_java_io_FileOutputStream_openAppend(JNIEnv *env, jobject this, jstring path) {
- fileOpen(env, this, path, fos_fd, O_WRONLY | O_CREAT | O_APPEND);
+Java_java_io_FileOutputStream_open(JNIEnv *env, jobject this,
+ jstring path, jboolean append) {
+ fileOpen(env, this, path, fos_fd,
+ O_WRONLY | O_CREAT | (append ? O_APPEND : O_TRUNC));
}
JNIEXPORT void JNICALL
--- a/jdk/src/windows/native/java/io/FileOutputStream_md.c Mon Mar 10 14:32:51 2008 -0700
+++ b/jdk/src/windows/native/java/io/FileOutputStream_md.c Mon Mar 10 14:32:51 2008 -0700
@@ -39,8 +39,6 @@
jfieldID fos_fd; /* id for jobject 'fd' in java.io.FileOutputStream */
-jfieldID fos_append;
-
/**************************************************************
* static methods to store field ID's in initializers
*/
@@ -49,7 +47,6 @@
Java_java_io_FileOutputStream_initIDs(JNIEnv *env, jclass fosClass) {
fos_fd =
(*env)->GetFieldID(env, fosClass, "fd", "Ljava/io/FileDescriptor;");
- fos_append = (*env)->GetFieldID(env, fosClass, "append", "Z");
}
/**************************************************************
@@ -57,45 +54,20 @@
*/
JNIEXPORT void JNICALL
-Java_java_io_FileOutputStream_open(JNIEnv *env, jobject this, jstring path) {
- fileOpen(env, this, path, fos_fd, O_WRONLY | O_CREAT | O_TRUNC);
-}
-
-JNIEXPORT void JNICALL
-Java_java_io_FileOutputStream_openAppend(JNIEnv *env, jobject this, jstring path) {
- fileOpen(env, this, path, fos_fd, O_WRONLY | O_CREAT | O_APPEND);
+Java_java_io_FileOutputStream_open(JNIEnv *env, jobject this,
+ jstring path, jboolean append) {
+ fileOpen(env, this, path, fos_fd,
+ O_WRONLY | O_CREAT | (append ? O_APPEND : O_TRUNC));
}
JNIEXPORT void JNICALL
Java_java_io_FileOutputStream_write(JNIEnv *env, jobject this, jint byte) {
- jboolean append = (*env)->GetBooleanField(env, this, fos_append);
- FD fd = GET_FD(this, fos_fd);
- if (fd == -1) {
- JNU_ThrowIOException(env, "Stream Closed");
- return;
- }
- if (append == JNI_TRUE) {
- if (IO_Lseek(fd, 0L, SEEK_END) == -1) {
- JNU_ThrowIOExceptionWithLastError(env, "Append failed");
- }
- }
writeSingle(env, this, byte, fos_fd);
}
JNIEXPORT void JNICALL
Java_java_io_FileOutputStream_writeBytes(JNIEnv *env,
jobject this, jbyteArray bytes, jint off, jint len) {
- jboolean append = (*env)->GetBooleanField(env, this, fos_append);
- FD fd = GET_FD(this, fos_fd);
- if (fd == -1) {
- JNU_ThrowIOException(env, "Stream Closed");
- return;
- }
- if (append == JNI_TRUE) {
- if (IO_Lseek(fd, 0L, SEEK_END) == -1) {
- JNU_ThrowIOExceptionWithLastError(env, "Append failed");
- }
- }
writeBytes(env, this, bytes, off, len, fos_fd);
}
--- a/jdk/src/windows/native/java/io/io_util_md.c Mon Mar 10 14:32:51 2008 -0700
+++ b/jdk/src/windows/native/java/io/io_util_md.c Mon Mar 10 14:32:51 2008 -0700
@@ -42,7 +42,7 @@
extern jboolean onNT = JNI_FALSE;
-static int MAX_INPUT_EVENTS = 2000;
+static DWORD MAX_INPUT_EVENTS = 2000;
void
initializeWindowsVersion() {
@@ -190,9 +190,16 @@
jlong
winFileHandleOpen(JNIEnv *env, jstring path, int flags)
{
+ /* To implement O_APPEND, we use the strategy from
+ http://msdn2.microsoft.com/en-us/library/aa363858.aspx
+ "You can get atomic append by opening a file with
+ FILE_APPEND_DATA access and _without_ FILE_WRITE_DATA access.
+ If you do this then all writes will ignore the current file
+ pointer and be done at the end-of file." */
const DWORD access =
- (flags & O_RDWR) ? (GENERIC_WRITE | GENERIC_READ) :
+ (flags & O_APPEND) ? (FILE_GENERIC_WRITE & ~FILE_WRITE_DATA) :
(flags & O_WRONLY) ? GENERIC_WRITE :
+ (flags & O_RDWR) ? (GENERIC_READ | GENERIC_WRITE) :
GENERIC_READ;
const DWORD sharing =
FILE_SHARE_READ | FILE_SHARE_WRITE;
@@ -495,7 +502,7 @@
FD fd = GET_FD(this, fid);
HANDLE h = (HANDLE)fd;
- if (fd == INVALID_HANDLE_VALUE) {
+ if (h == INVALID_HANDLE_VALUE) {
return 0;
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/io/FileOutputStream/AtomicAppend.java Mon Mar 10 14:32:51 2008 -0700
@@ -0,0 +1,81 @@
+/*
+ * Copyright 2007 Sun Microsystems, Inc. All Rights Reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+/*
+ * @test
+ * @bug 6631352
+ * @summary Check that appends are atomic
+ */
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.TimeUnit;
+
+public class AtomicAppend {
+ // Before the fix for
+ // 6631352: Implement atomic append mode using FILE_APPEND_DATA (win)
+ // this would fail intermittently on windows
+ void test(String[] args) throws Throwable {
+ final int nThreads = 10;
+ final int writes = 1000;
+ final File file = new File("foo");
+ file.delete();
+ try {
+ final ExecutorService es = Executors.newFixedThreadPool(nThreads);
+ for (int i = 0; i < nThreads; i++)
+ es.execute(new Runnable() { public void run() {
+ try {
+ FileOutputStream s = new FileOutputStream(file, true);
+ for (int j = 0; j < 1000; j++) {
+ s.write((int) 'x');
+ s.flush();
+ }
+ s.close();
+ } catch (Throwable t) { unexpected(t); }}});
+ es.shutdown();
+ es.awaitTermination(10L, TimeUnit.MINUTES);
+ equal(file.length(), (long) (nThreads * writes));
+ } finally {
+ file.delete();
+ }
+ }
+
+ //--------------------- Infrastructure ---------------------------
+ volatile int passed = 0, failed = 0;
+ void pass() {passed++;}
+ void fail() {failed++; Thread.dumpStack();}
+ void fail(String msg) {System.err.println(msg); fail();}
+ void unexpected(Throwable t) {failed++; t.printStackTrace();}
+ void check(boolean cond) {if (cond) pass(); else fail();}
+ void equal(Object x, Object y) {
+ if (x == null ? y == null : x.equals(y)) pass();
+ else fail(x + " not equal to " + y);}
+ public static void main(String[] args) throws Throwable {
+ new AtomicAppend().instanceMain(args);}
+ void instanceMain(String[] args) throws Throwable {
+ try {test(args);} catch (Throwable t) {unexpected(t);}
+ System.out.printf("%nPassed = %d, failed = %d%n%n", passed, failed);
+ if (failed > 0) throw new AssertionError("Some tests failed");}
+}