6913877: (fs) AsynchronousFileChannel.write can return wrong result under load [win]
Reviewed-by: chegar
--- a/jdk/src/windows/classes/sun/nio/ch/WindowsAsynchronousFileChannelImpl.java Sat Jan 09 19:32:58 2010 +0000
+++ b/jdk/src/windows/classes/sun/nio/ch/WindowsAsynchronousFileChannelImpl.java Sun Jan 10 12:29:19 2010 +0000
@@ -445,20 +445,17 @@
// allocate OVERLAPPED
overlapped = ioCache.add(result);
- // synchronize on result to allow this thread handle the case
- // where the read completes immediately.
- synchronized (result) {
- n = readFile(handle, address, rem, position, overlapped);
- if (n == IOStatus.UNAVAILABLE) {
- // I/O is pending
- return;
- }
- // read completed immediately:
- // 1. update buffer position
- // 2. release waiters
- updatePosition(n);
+ // initiate read
+ n = readFile(handle, address, rem, position, overlapped);
+ if (n == IOStatus.UNAVAILABLE) {
+ // I/O is pending
+ return;
+ } else if (n == IOStatus.EOF) {
result.setResult(n);
+ } else {
+ throw new InternalError("Unexpected result: " + n);
}
+
} catch (Throwable x) {
// failed to initiate read
result.setFailure(toIOException(x));
@@ -466,12 +463,9 @@
end();
}
- // read failed or EOF so completion port will not be notified
- if (n < 0 && overlapped != 0L) {
+ // release resources
+ if (overlapped != 0L)
ioCache.remove(overlapped);
- }
-
- // return direct buffer to cache if substituted
releaseBufferIfSubstituted();
// invoke completion handler
@@ -634,20 +628,15 @@
// allocate an OVERLAPPED structure
overlapped = ioCache.add(result);
- // synchronize on result to allow this thread handle the case
- // where the read completes immediately.
- synchronized (result) {
- n = writeFile(handle, address, rem, position, overlapped);
- if (n == IOStatus.UNAVAILABLE) {
- // I/O is pending
- return;
- }
- // read completed immediately:
- // 1. update buffer position
- // 2. release waiters
- updatePosition(n);
- result.setResult(n);
+ // initiate the write
+ n = writeFile(handle, address, rem, position, overlapped);
+ if (n == IOStatus.UNAVAILABLE) {
+ // I/O is pending
+ return;
+ } else {
+ throw new InternalError("Unexpected result: " + n);
}
+
} catch (Throwable x) {
// failed to initiate read:
result.setFailure(toIOException(x));
--- a/jdk/src/windows/native/sun/nio/ch/WindowsAsynchronousFileChannelImpl.c Sat Jan 09 19:32:58 2010 +0000
+++ b/jdk/src/windows/native/sun/nio/ch/WindowsAsynchronousFileChannelImpl.c Sun Jan 10 12:29:19 2010 +0000
@@ -39,7 +39,6 @@
jlong handle, jlong address, jint len, jlong offset, jlong ov)
{
BOOL res;
- DWORD nread = 0;
OVERLAPPED* lpOverlapped = (OVERLAPPED*)jlong_to_ptr(ov);
lpOverlapped->Offset = (DWORD)offset;
@@ -49,7 +48,7 @@
res = ReadFile((HANDLE) jlong_to_ptr(handle),
(LPVOID) jlong_to_ptr(address),
(DWORD)len,
- &nread,
+ NULL,
lpOverlapped);
if (res == 0) {
@@ -62,7 +61,7 @@
return IOS_THROWN;
}
- return (jint)nread;
+ return IOS_UNAVAILABLE;
}
JNIEXPORT jint JNICALL
@@ -70,7 +69,6 @@
jlong handle, jlong address, jint len, jlong offset, jlong ov)
{
BOOL res;
- DWORD nwritten = 0;
OVERLAPPED* lpOverlapped = (OVERLAPPED*)jlong_to_ptr(ov);
lpOverlapped->Offset = (DWORD)offset;
@@ -80,18 +78,18 @@
res = WriteFile((HANDLE)jlong_to_ptr(handle),
(LPVOID) jlong_to_ptr(address),
(DWORD)len,
- &nwritten,
+ NULL,
lpOverlapped);
if (res == 0) {
int error = GetLastError();
- if (error == ERROR_IO_PENDING) {
+ if (error == ERROR_IO_PENDING)
return IOS_UNAVAILABLE;
- }
JNU_ThrowIOExceptionWithLastError(env, "WriteFile failed");
return IOS_THROWN;
}
- return (jint)nwritten;
+
+ return IOS_UNAVAILABLE;
}
JNIEXPORT jint JNICALL
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/nio/channels/AsynchronousFileChannel/LotsOfWrites.java Sun Jan 10 12:29:19 2010 +0000
@@ -0,0 +1,162 @@
+/*
+ * Copyright 2010 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 6913877
+ * @summary Stress AsynchronousFileChannel.write
+ */
+
+import java.io.*;
+import java.nio.ByteBuffer;
+import static java.nio.file.StandardOpenOption.*;
+import java.nio.channels.*;
+import java.util.Random;
+import java.util.concurrent.CountDownLatch;
+
+public class LotsOfWrites {
+ static final Random rand = new Random();
+
+ /**
+ * Asynchronously writes a known pattern to a file up to a given size,
+ * counting down a latch to release waiters when done.
+ */
+ static class Writer implements CompletionHandler<Integer,ByteBuffer> {
+ private final File file;
+ private final long size;
+ private final CountDownLatch latch;
+ private final AsynchronousFileChannel channel;
+
+ private volatile long position;
+ private volatile byte nextByte;
+
+ private long updatePosition(long nwrote) {
+ position += nwrote;
+ return position;
+ }
+
+ private ByteBuffer genNextBuffer() {
+ int n = Math.min(8192 + rand.nextInt(8192), (int)(size - position));
+ ByteBuffer buf = ByteBuffer.allocate(n);
+ for (int i=0; i<n; i++) {
+ buf.put(nextByte++);
+ }
+ buf.flip();
+ return buf;
+ }
+
+ // close channel and release any waiters
+ private void done() {
+ try {
+ channel.close();
+ } catch (IOException ignore) { }
+ latch.countDown();
+ }
+
+ Writer(File file, long size, CountDownLatch latch) throws IOException {
+ this.file = file;
+ this.size = size;
+ this.latch = latch;
+ this.channel = AsynchronousFileChannel.open(file.toPath(), WRITE);
+ }
+
+ File file() {
+ return file;
+ }
+
+ long size() {
+ return size;
+ }
+
+ // initiate first write
+ void start() {
+ ByteBuffer buf = genNextBuffer();
+ channel.write(buf, 0L, buf, this);
+ }
+
+ @Override
+ public void completed(Integer nwrote, ByteBuffer buf) {
+ long pos = updatePosition(nwrote);
+ if (!buf.hasRemaining()) {
+ // buffer has been completely written; decide if we need to
+ // write more
+ if (position >= size) {
+ done();
+ return;
+ }
+ buf = genNextBuffer();
+ }
+ channel.write(buf, pos, buf, this);
+ }
+
+ @Override
+ public void failed(Throwable exc, ByteBuffer buf) {
+ exc.printStackTrace();
+ done();
+ }
+ }
+
+ public static void main(String[] args) throws Exception {
+ // random number of writers
+ int count = 20 + rand.nextInt(16);
+ Writer[] writers = new Writer[count];
+ CountDownLatch latch = new CountDownLatch(count);
+
+ // initiate writing to each file
+ for (int i=0; i<count; i++) {
+ long size = 512*1024 + rand.nextInt(512*1024);
+ File blah = File.createTempFile("blah", null);
+ blah.deleteOnExit();
+ Writer writer = new Writer(blah, size, latch);
+ writers[i] = writer;
+ writer.start();
+ }
+
+ // wait for writing to complete
+ latch.await();
+
+ // verify content of each file
+ byte[] buf = new byte[8192];
+ for (int i=0; i<count ;i++) {
+ Writer writer = writers[i];
+ FileInputStream in = new FileInputStream(writer.file());
+ try {
+ long size = 0L;
+ byte expected = 0;
+ int nread = in.read(buf);
+ while (nread > 0) {
+ for (int j=0; j<nread; j++) {
+ if (buf[j] != expected)
+ throw new RuntimeException("Unexpected byte");
+ expected++;
+ }
+ size += nread;
+ nread = in.read(buf);
+ }
+ if (size != writer.size())
+ throw new RuntimeException("Unexpected size");
+ } finally {
+ in.close();
+ }
+ }
+ }
+}