8012019: (fc) Thread.interrupt triggers hang in FileChannelImpl.pread (win)
Reviewed-by: chegar
--- a/jdk/src/share/classes/sun/nio/ch/DatagramChannelImpl.java Wed Apr 17 02:53:02 2013 -0700
+++ b/jdk/src/share/classes/sun/nio/ch/DatagramChannelImpl.java Wed Apr 17 16:11:19 2013 +0100
@@ -538,7 +538,7 @@
return 0;
readerThread = NativeThread.current();
do {
- n = IOUtil.read(fd, buf, -1, nd, readLock);
+ n = IOUtil.read(fd, buf, -1, nd);
} while ((n == IOStatus.INTERRUPTED) && isOpen());
return IOStatus.normalize(n);
} finally {
@@ -594,7 +594,7 @@
return 0;
writerThread = NativeThread.current();
do {
- n = IOUtil.write(fd, buf, -1, nd, writeLock);
+ n = IOUtil.write(fd, buf, -1, nd);
} while ((n == IOStatus.INTERRUPTED) && isOpen());
return IOStatus.normalize(n);
} finally {
--- a/jdk/src/share/classes/sun/nio/ch/FileChannelImpl.java Wed Apr 17 02:53:02 2013 -0700
+++ b/jdk/src/share/classes/sun/nio/ch/FileChannelImpl.java Wed Apr 17 16:11:19 2013 +0100
@@ -140,7 +140,7 @@
if (!isOpen())
return 0;
do {
- n = IOUtil.read(fd, dst, -1, nd, positionLock);
+ n = IOUtil.read(fd, dst, -1, nd);
} while ((n == IOStatus.INTERRUPTED) && isOpen());
return IOStatus.normalize(n);
} finally {
@@ -192,7 +192,7 @@
if (!isOpen())
return 0;
do {
- n = IOUtil.write(fd, src, -1, nd, positionLock);
+ n = IOUtil.write(fd, src, -1, nd);
} while ((n == IOStatus.INTERRUPTED) && isOpen());
return IOStatus.normalize(n);
} finally {
@@ -671,6 +671,17 @@
if (!readable)
throw new NonReadableChannelException();
ensureOpen();
+ if (nd.needsPositionLock()) {
+ synchronized (positionLock) {
+ return readInternal(dst, position);
+ }
+ } else {
+ return readInternal(dst, position);
+ }
+ }
+
+ private int readInternal(ByteBuffer dst, long position) throws IOException {
+ assert !nd.needsPositionLock() || Thread.holdsLock(positionLock);
int n = 0;
int ti = -1;
try {
@@ -679,7 +690,7 @@
if (!isOpen())
return -1;
do {
- n = IOUtil.read(fd, dst, position, nd, positionLock);
+ n = IOUtil.read(fd, dst, position, nd);
} while ((n == IOStatus.INTERRUPTED) && isOpen());
return IOStatus.normalize(n);
} finally {
@@ -697,6 +708,17 @@
if (!writable)
throw new NonWritableChannelException();
ensureOpen();
+ if (nd.needsPositionLock()) {
+ synchronized (positionLock) {
+ return writeInternal(src, position);
+ }
+ } else {
+ return writeInternal(src, position);
+ }
+ }
+
+ private int writeInternal(ByteBuffer src, long position) throws IOException {
+ assert !nd.needsPositionLock() || Thread.holdsLock(positionLock);
int n = 0;
int ti = -1;
try {
@@ -705,7 +727,7 @@
if (!isOpen())
return -1;
do {
- n = IOUtil.write(fd, src, position, nd, positionLock);
+ n = IOUtil.write(fd, src, position, nd);
} while ((n == IOStatus.INTERRUPTED) && isOpen());
return IOStatus.normalize(n);
} finally {
--- a/jdk/src/share/classes/sun/nio/ch/IOUtil.java Wed Apr 17 02:53:02 2013 -0700
+++ b/jdk/src/share/classes/sun/nio/ch/IOUtil.java Wed Apr 17 16:11:19 2013 +0100
@@ -44,11 +44,11 @@
private IOUtil() { } // No instantiation
static int write(FileDescriptor fd, ByteBuffer src, long position,
- NativeDispatcher nd, Object lock)
+ NativeDispatcher nd)
throws IOException
{
if (src instanceof DirectBuffer)
- return writeFromNativeBuffer(fd, src, position, nd, lock);
+ return writeFromNativeBuffer(fd, src, position, nd);
// Substitute a native buffer
int pos = src.position();
@@ -62,7 +62,7 @@
// Do not update src until we see how many bytes were written
src.position(pos);
- int n = writeFromNativeBuffer(fd, bb, position, nd, lock);
+ int n = writeFromNativeBuffer(fd, bb, position, nd);
if (n > 0) {
// now update src
src.position(pos + n);
@@ -74,8 +74,7 @@
}
private static int writeFromNativeBuffer(FileDescriptor fd, ByteBuffer bb,
- long position, NativeDispatcher nd,
- Object lock)
+ long position, NativeDispatcher nd)
throws IOException
{
int pos = bb.position();
@@ -89,7 +88,7 @@
if (position != -1) {
written = nd.pwrite(fd,
((DirectBuffer)bb).address() + pos,
- rem, position, lock);
+ rem, position);
} else {
written = nd.write(fd, ((DirectBuffer)bb).address() + pos, rem);
}
@@ -184,18 +183,18 @@
}
static int read(FileDescriptor fd, ByteBuffer dst, long position,
- NativeDispatcher nd, Object lock)
+ NativeDispatcher nd)
throws IOException
{
if (dst.isReadOnly())
throw new IllegalArgumentException("Read-only buffer");
if (dst instanceof DirectBuffer)
- return readIntoNativeBuffer(fd, dst, position, nd, lock);
+ return readIntoNativeBuffer(fd, dst, position, nd);
// Substitute a native buffer
ByteBuffer bb = Util.getTemporaryDirectBuffer(dst.remaining());
try {
- int n = readIntoNativeBuffer(fd, bb, position, nd, lock);
+ int n = readIntoNativeBuffer(fd, bb, position, nd);
bb.flip();
if (n > 0)
dst.put(bb);
@@ -206,8 +205,7 @@
}
private static int readIntoNativeBuffer(FileDescriptor fd, ByteBuffer bb,
- long position, NativeDispatcher nd,
- Object lock)
+ long position, NativeDispatcher nd)
throws IOException
{
int pos = bb.position();
@@ -220,7 +218,7 @@
int n = 0;
if (position != -1) {
n = nd.pread(fd, ((DirectBuffer)bb).address() + pos,
- rem, position, lock);
+ rem, position);
} else {
n = nd.read(fd, ((DirectBuffer)bb).address() + pos, rem);
}
--- a/jdk/src/share/classes/sun/nio/ch/NativeDispatcher.java Wed Apr 17 02:53:02 2013 -0700
+++ b/jdk/src/share/classes/sun/nio/ch/NativeDispatcher.java Wed Apr 17 16:11:19 2013 +0100
@@ -38,8 +38,16 @@
abstract int read(FileDescriptor fd, long address, int len)
throws IOException;
- int pread(FileDescriptor fd, long address, int len,
- long position, Object lock) throws IOException
+ /**
+ * Returns {@code true} if pread/pwrite needs to be synchronized with
+ * position sensitive methods.
+ */
+ boolean needsPositionLock() {
+ return false;
+ }
+
+ int pread(FileDescriptor fd, long address, int len, long position)
+ throws IOException
{
throw new IOException("Operation Unsupported");
}
@@ -50,8 +58,8 @@
abstract int write(FileDescriptor fd, long address, int len)
throws IOException;
- int pwrite(FileDescriptor fd, long address, int len,
- long position, Object lock) throws IOException
+ int pwrite(FileDescriptor fd, long address, int len, long position)
+ throws IOException
{
throw new IOException("Operation Unsupported");
}
--- a/jdk/src/share/classes/sun/nio/ch/SimpleAsynchronousFileChannelImpl.java Wed Apr 17 02:53:02 2013 -0700
+++ b/jdk/src/share/classes/sun/nio/ch/SimpleAsynchronousFileChannelImpl.java Wed Apr 17 16:11:19 2013 +0100
@@ -318,7 +318,7 @@
try {
begin();
do {
- n = IOUtil.read(fdObj, dst, position, nd, null);
+ n = IOUtil.read(fdObj, dst, position, nd);
} while ((n == IOStatus.INTERRUPTED) && isOpen());
if (n < 0 && !isOpen())
throw new AsynchronousCloseException();
@@ -372,7 +372,7 @@
try {
begin();
do {
- n = IOUtil.write(fdObj, src, position, nd, null);
+ n = IOUtil.write(fdObj, src, position, nd);
} while ((n == IOStatus.INTERRUPTED) && isOpen());
if (n < 0 && !isOpen())
throw new AsynchronousCloseException();
--- a/jdk/src/share/classes/sun/nio/ch/SocketChannelImpl.java Wed Apr 17 02:53:02 2013 -0700
+++ b/jdk/src/share/classes/sun/nio/ch/SocketChannelImpl.java Wed Apr 17 16:11:19 2013 +0100
@@ -356,7 +356,7 @@
// except that the shutdown operation plays the role of
// nd.preClose().
for (;;) {
- n = IOUtil.read(fd, buf, -1, nd, readLock);
+ n = IOUtil.read(fd, buf, -1, nd);
if ((n == IOStatus.INTERRUPTED) && isOpen()) {
// The system call was interrupted but the channel
// is still open, so retry
@@ -447,7 +447,7 @@
writerThread = NativeThread.current();
}
for (;;) {
- n = IOUtil.write(fd, buf, -1, nd, writeLock);
+ n = IOUtil.write(fd, buf, -1, nd);
if ((n == IOStatus.INTERRUPTED) && isOpen())
continue;
return IOStatus.normalize(n);
--- a/jdk/src/solaris/classes/sun/nio/ch/FileDispatcherImpl.java Wed Apr 17 02:53:02 2013 -0700
+++ b/jdk/src/solaris/classes/sun/nio/ch/FileDispatcherImpl.java Wed Apr 17 16:11:19 2013 +0100
@@ -46,8 +46,9 @@
return read0(fd, address, len);
}
- int pread(FileDescriptor fd, long address, int len,
- long position, Object lock) throws IOException {
+ int pread(FileDescriptor fd, long address, int len, long position)
+ throws IOException
+ {
return pread0(fd, address, len, position);
}
@@ -59,8 +60,8 @@
return write0(fd, address, len);
}
- int pwrite(FileDescriptor fd, long address, int len,
- long position, Object lock) throws IOException
+ int pwrite(FileDescriptor fd, long address, int len, long position)
+ throws IOException
{
return pwrite0(fd, address, len, position);
}
--- a/jdk/src/solaris/classes/sun/nio/ch/SinkChannelImpl.java Wed Apr 17 02:53:02 2013 -0700
+++ b/jdk/src/solaris/classes/sun/nio/ch/SinkChannelImpl.java Wed Apr 17 16:11:19 2013 +0100
@@ -165,7 +165,7 @@
return 0;
thread = NativeThread.current();
do {
- n = IOUtil.write(fd, src, -1, nd, lock);
+ n = IOUtil.write(fd, src, -1, nd);
} while ((n == IOStatus.INTERRUPTED) && isOpen());
return IOStatus.normalize(n);
} finally {
--- a/jdk/src/solaris/classes/sun/nio/ch/SourceChannelImpl.java Wed Apr 17 02:53:02 2013 -0700
+++ b/jdk/src/solaris/classes/sun/nio/ch/SourceChannelImpl.java Wed Apr 17 16:11:19 2013 +0100
@@ -165,7 +165,7 @@
return 0;
thread = NativeThread.current();
do {
- n = IOUtil.read(fd, dst, -1, nd, lock);
+ n = IOUtil.read(fd, dst, -1, nd);
} while ((n == IOStatus.INTERRUPTED) && isOpen());
return IOStatus.normalize(n);
} finally {
--- a/jdk/src/solaris/classes/sun/nio/ch/UnixAsynchronousSocketChannelImpl.java Wed Apr 17 02:53:02 2013 -0700
+++ b/jdk/src/solaris/classes/sun/nio/ch/UnixAsynchronousSocketChannelImpl.java Wed Apr 17 16:11:19 2013 +0100
@@ -384,7 +384,7 @@
if (scattering) {
n = (int)IOUtil.read(fd, readBuffers, nd);
} else {
- n = IOUtil.read(fd, readBuffer, -1, nd, null);
+ n = IOUtil.read(fd, readBuffer, -1, nd);
}
if (n == IOStatus.UNAVAILABLE) {
// spurious wakeup, is this possible?
@@ -505,7 +505,7 @@
if (isScatteringRead) {
n = (int)IOUtil.read(fd, dsts, nd);
} else {
- n = IOUtil.read(fd, dst, -1, nd, null);
+ n = IOUtil.read(fd, dst, -1, nd);
}
}
@@ -579,7 +579,7 @@
if (gathering) {
n = (int)IOUtil.write(fd, writeBuffers, nd);
} else {
- n = IOUtil.write(fd, writeBuffer, -1, nd, null);
+ n = IOUtil.write(fd, writeBuffer, -1, nd);
}
if (n == IOStatus.UNAVAILABLE) {
// spurious wakeup, is this possible?
@@ -688,7 +688,7 @@
if (isGatheringWrite) {
n = (int)IOUtil.write(fd, srcs, nd);
} else {
- n = IOUtil.write(fd, src, -1, nd, null);
+ n = IOUtil.write(fd, src, -1, nd);
}
}
--- a/jdk/src/windows/classes/sun/nio/ch/FileDispatcherImpl.java Wed Apr 17 02:53:02 2013 -0700
+++ b/jdk/src/windows/classes/sun/nio/ch/FileDispatcherImpl.java Wed Apr 17 16:11:19 2013 +0100
@@ -49,18 +49,21 @@
this(false);
}
+ @Override
+ boolean needsPositionLock() {
+ return true;
+ }
+
int read(FileDescriptor fd, long address, int len)
throws IOException
{
return read0(fd, address, len);
}
- int pread(FileDescriptor fd, long address, int len,
- long position, Object lock) throws IOException
+ int pread(FileDescriptor fd, long address, int len, long position)
+ throws IOException
{
- synchronized(lock) {
- return pread0(fd, address, len, position);
- }
+ return pread0(fd, address, len, position);
}
long readv(FileDescriptor fd, long address, int len) throws IOException {
@@ -71,12 +74,10 @@
return write0(fd, address, len, append);
}
- int pwrite(FileDescriptor fd, long address, int len,
- long position, Object lock) throws IOException
+ int pwrite(FileDescriptor fd, long address, int len, long position)
+ throws IOException
{
- synchronized(lock) {
- return pwrite0(fd, address, len, position);
- }
+ return pwrite0(fd, address, len, position);
}
long writev(FileDescriptor fd, long address, int len) throws IOException {
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/nio/channels/FileChannel/InterruptDeadlock.java Wed Apr 17 16:11:19 2013 +0100
@@ -0,0 +1,137 @@
+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. 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 Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/* @test
+ * @bug 8012019
+ * @summary Tests interruption of threads doing position-based read methods in
+ * an attempt to provoke a deadlock between position sensitive and position
+ * insensitive methods
+ */
+import java.nio.ByteBuffer;
+import java.nio.channels.*;
+import java.nio.file.*;
+import static java.nio.file.StandardOpenOption.*;
+
+public class InterruptDeadlock {
+
+ /**
+ * A thread that continuously reads from a FileChannel with
+ * read(ByteBuffer,long). The thread terminates when interrupted and/or
+ * the FileChannel is closed.
+ */
+ static class Reader extends Thread {
+ final FileChannel fc;
+ volatile Exception exception;
+
+ Reader(FileChannel fc) {
+ this.fc = fc;
+ }
+
+ @Override
+ public void run() {
+ ByteBuffer bb = ByteBuffer.allocate(1024);
+ try {
+ long pos = 0L;
+ for (;;) {
+ bb.clear();
+ int n = fc.read(bb, pos);
+ if (n > 0)
+ pos += n;
+ // fc.size is important here as it is position sensitive
+ if (pos > fc.size())
+ pos = 0L;
+ }
+ } catch (ClosedChannelException x) {
+ System.out.println(x.getClass() + " (expected)");
+ } catch (Exception unexpected) {
+ this.exception = unexpected;
+ }
+ }
+
+ Exception exception() {
+ return exception;
+ }
+
+ static Reader startReader(FileChannel fc) {
+ Reader r = new Reader(fc);
+ r.start();
+ return r;
+ }
+ }
+
+ // the number of reader threads to start
+ private static final int READER_COUNT = 4;
+
+ public static void main(String[] args) throws Exception {
+ Path file = Paths.get("data.txt");
+ try (FileChannel fc = FileChannel.open(file, CREATE, TRUNCATE_EXISTING, WRITE)) {
+ fc.position(1024L * 1024L);
+ fc.write(ByteBuffer.wrap(new byte[1]));
+ }
+
+ Reader[] readers = new Reader[READER_COUNT];
+
+ for (int i=1; i<=20; i++) {
+ System.out.format("Iteration: %s%n", i);
+
+ try (FileChannel fc = FileChannel.open(file)) {
+ boolean failed = false;
+
+ // start reader threads
+ for (int j=0; j<READER_COUNT; j++) {
+ readers[j] = Reader.startReader(fc);
+ }
+
+ // give readers a bit of time to get started (not strictly required)
+ Thread.sleep(100);
+
+ // interrupt and wait for the readers to terminate
+ for (Reader r: readers) {
+ r.interrupt();
+ }
+ for (Reader r: readers) {
+ try {
+ r.join(10000);
+ Exception e = r.exception();
+ if (e != null) {
+ System.err.println("Reader thread failed with: " + e);
+ failed = true;
+ }
+ } catch (InterruptedException x) {
+ System.err.println("Reader thread did not terminte");
+ failed = true;
+ }
+ }
+
+ // the channel should not be open at this point
+ if (fc.isOpen()) {
+ System.err.println("FileChannel was not closed");
+ failed = true;
+ }
+
+ if (failed)
+ throw new RuntimeException("Test failed - see log for details");
+ }
+ }
+ }
+}