diff --git a/src/share/classes/sun/nio/ch/DatagramChannelImpl.java b/src/share/classes/sun/nio/ch/DatagramChannelImpl.java index cdd5b3c2a806c2fdfd8941735001230c447a4ab9..4e41a9aa1b80bd715e023c842f6873596481e535 100644 --- a/src/share/classes/sun/nio/ch/DatagramChannelImpl.java +++ b/src/share/classes/sun/nio/ch/DatagramChannelImpl.java @@ -538,7 +538,7 @@ class DatagramChannelImpl 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 @@ class DatagramChannelImpl 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 { diff --git a/src/share/classes/sun/nio/ch/FileChannelImpl.java b/src/share/classes/sun/nio/ch/FileChannelImpl.java index a9b9791666bc52d9fbd0aa8ca1703d72c167f1fb..ac9be90f2106a6c9a73346eca130d68f6fc54254 100644 --- a/src/share/classes/sun/nio/ch/FileChannelImpl.java +++ b/src/share/classes/sun/nio/ch/FileChannelImpl.java @@ -140,7 +140,7 @@ public class FileChannelImpl 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 @@ public class FileChannelImpl 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 @@ public class FileChannelImpl 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 @@ public class FileChannelImpl 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 @@ public class FileChannelImpl 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 @@ public class FileChannelImpl 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 { diff --git a/src/share/classes/sun/nio/ch/IOUtil.java b/src/share/classes/sun/nio/ch/IOUtil.java index 7846486ecc8e14babbc0f2c6ed9350ae20c52e3f..6f45da73d1060e499f15cfd3fd81321ce20b1796 100644 --- a/src/share/classes/sun/nio/ch/IOUtil.java +++ b/src/share/classes/sun/nio/ch/IOUtil.java @@ -44,11 +44,11 @@ public class IOUtil { 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 @@ public class IOUtil { // 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 @@ public class IOUtil { } 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 @@ public class IOUtil { 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 @@ public class IOUtil { } 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 @@ public class IOUtil { } 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 @@ public class IOUtil { 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); } diff --git a/src/share/classes/sun/nio/ch/NativeDispatcher.java b/src/share/classes/sun/nio/ch/NativeDispatcher.java index 4752ce8e389426bcd84040aebe1052a51ce22358..bcc507d549ef83e68cbcdeb50c7b59d1fa9480b4 100644 --- a/src/share/classes/sun/nio/ch/NativeDispatcher.java +++ b/src/share/classes/sun/nio/ch/NativeDispatcher.java @@ -38,8 +38,16 @@ abstract class NativeDispatcher 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 class NativeDispatcher 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"); } diff --git a/src/share/classes/sun/nio/ch/SimpleAsynchronousFileChannelImpl.java b/src/share/classes/sun/nio/ch/SimpleAsynchronousFileChannelImpl.java index ede531bd0240bb19f364219c4944665642bf8dcf..741abf733fd94b1c5a325d50d6d827fe8332afa9 100644 --- a/src/share/classes/sun/nio/ch/SimpleAsynchronousFileChannelImpl.java +++ b/src/share/classes/sun/nio/ch/SimpleAsynchronousFileChannelImpl.java @@ -318,7 +318,7 @@ public class SimpleAsynchronousFileChannelImpl 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 @@ public class SimpleAsynchronousFileChannelImpl 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(); diff --git a/src/share/classes/sun/nio/ch/SocketChannelImpl.java b/src/share/classes/sun/nio/ch/SocketChannelImpl.java index 9696038ecbf29d2cef5f216b48fe321f5ad13cc6..22be8fb6efdd2a946f0d0e525af7ade55896b20b 100644 --- a/src/share/classes/sun/nio/ch/SocketChannelImpl.java +++ b/src/share/classes/sun/nio/ch/SocketChannelImpl.java @@ -356,7 +356,7 @@ class SocketChannelImpl // 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 @@ class SocketChannelImpl 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); diff --git a/src/solaris/classes/sun/nio/ch/FileDispatcherImpl.java b/src/solaris/classes/sun/nio/ch/FileDispatcherImpl.java index 79f238dc7de4553cd6c13b9d7587e2a5cdb84efb..37c1d5b9098032ed0ae1ccfa01ac695d7163b723 100644 --- a/src/solaris/classes/sun/nio/ch/FileDispatcherImpl.java +++ b/src/solaris/classes/sun/nio/ch/FileDispatcherImpl.java @@ -46,8 +46,9 @@ class FileDispatcherImpl extends FileDispatcher 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 @@ class FileDispatcherImpl extends FileDispatcher 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); } diff --git a/src/solaris/classes/sun/nio/ch/SinkChannelImpl.java b/src/solaris/classes/sun/nio/ch/SinkChannelImpl.java index e99c184f8e9999af3f02936384d5c9adae406b25..d388dd47fd842f4bb46497f80e1acc1f1fe4eeb8 100644 --- a/src/solaris/classes/sun/nio/ch/SinkChannelImpl.java +++ b/src/solaris/classes/sun/nio/ch/SinkChannelImpl.java @@ -165,7 +165,7 @@ class SinkChannelImpl 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 { diff --git a/src/solaris/classes/sun/nio/ch/SourceChannelImpl.java b/src/solaris/classes/sun/nio/ch/SourceChannelImpl.java index b6b005ae315abb4b89c8b4bd62408c4f0ce5a59a..cc374127e97b7370f0431fd2c305013ed4cbd834 100644 --- a/src/solaris/classes/sun/nio/ch/SourceChannelImpl.java +++ b/src/solaris/classes/sun/nio/ch/SourceChannelImpl.java @@ -165,7 +165,7 @@ class SourceChannelImpl 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 { diff --git a/src/solaris/classes/sun/nio/ch/UnixAsynchronousSocketChannelImpl.java b/src/solaris/classes/sun/nio/ch/UnixAsynchronousSocketChannelImpl.java index d7fccc9d558b39c21f578ddcbdaf6060e1682c99..33859f03529614f2bfdeb9f8c5653488115ca2bf 100644 --- a/src/solaris/classes/sun/nio/ch/UnixAsynchronousSocketChannelImpl.java +++ b/src/solaris/classes/sun/nio/ch/UnixAsynchronousSocketChannelImpl.java @@ -384,7 +384,7 @@ class UnixAsynchronousSocketChannelImpl 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 @@ class UnixAsynchronousSocketChannelImpl 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 @@ class UnixAsynchronousSocketChannelImpl 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 @@ class UnixAsynchronousSocketChannelImpl 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); } } diff --git a/src/windows/classes/sun/nio/ch/FileDispatcherImpl.java b/src/windows/classes/sun/nio/ch/FileDispatcherImpl.java index 276c6199a1247fdb500ec43323e077af0226521d..de3c4f9f705e98ad52668bdc5a1f5289f1542a42 100644 --- a/src/windows/classes/sun/nio/ch/FileDispatcherImpl.java +++ b/src/windows/classes/sun/nio/ch/FileDispatcherImpl.java @@ -49,18 +49,21 @@ class FileDispatcherImpl extends FileDispatcher 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 @@ class FileDispatcherImpl extends FileDispatcher 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 { diff --git a/test/java/nio/channels/FileChannel/InterruptDeadlock.java b/test/java/nio/channels/FileChannel/InterruptDeadlock.java new file mode 100644 index 0000000000000000000000000000000000000000..102d7db900d2c67b9ec88aedf33e8c867c4049a9 --- /dev/null +++ b/test/java/nio/channels/FileChannel/InterruptDeadlock.java @@ -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