6913877: (fs) AsynchronousFileChannel.write can return wrong result under load [win]

Reviewed-by: chegar

src/windows/classes/sun/nio/ch/WindowsAsynchronousFileChannelImpl.java

@@ -445,20 +445,17 @@ public class WindowsAsynchronousFileChannelImpl
                 // 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 @@ public class WindowsAsynchronousFileChannelImpl
                 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 @@ public class WindowsAsynchronousFileChannelImpl
                 // 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));

src/windows/native/sun/nio/ch/WindowsAsynchronousFileChannelImpl.c

@@ -39,7 +39,6 @@ Java_sun_nio_ch_WindowsAsynchronousFileChannelImpl_readFile(JNIEnv* env, jclass
     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 @@ Java_sun_nio_ch_WindowsAsynchronousFileChannelImpl_readFile(JNIEnv* env, jclass
     res = ReadFile((HANDLE) jlong_to_ptr(handle),
                    (LPVOID) jlong_to_ptr(address),
                    (DWORD)len,
-                   &nread,
+                   NULL,
                    lpOverlapped);
 
     if (res == 0) {
@@ -62,7 +61,7 @@ Java_sun_nio_ch_WindowsAsynchronousFileChannelImpl_readFile(JNIEnv* env, jclass
         return IOS_THROWN;
     }
 
-    return (jint)nread;
+    return IOS_UNAVAILABLE;
 }
 
 JNIEXPORT jint JNICALL
@@ -70,7 +69,6 @@ Java_sun_nio_ch_WindowsAsynchronousFileChannelImpl_writeFile(JNIEnv* env, jclass
     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 @@ Java_sun_nio_ch_WindowsAsynchronousFileChannelImpl_writeFile(JNIEnv* env, jclass
     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

test/java/nio/channels/AsynchronousFileChannel/LotsOfWrites.java

+/*
+ * 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();
+            }
+        }
+    }
+}