Mercurial > hg > openjdk > jigsaw > jdk
changeset 7347:7ded74ffea36
8012019: (fc) Thread.interrupt triggers hang in FileChannelImpl.pread (win)
Reviewed-by: chegar
line wrap: on
line diff
--- a/src/share/classes/sun/nio/ch/DatagramChannelImpl.java Wed Apr 17 02:53:02 2013 -0700 +++ b/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/src/share/classes/sun/nio/ch/FileChannelImpl.java Wed Apr 17 02:53:02 2013 -0700 +++ b/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/src/share/classes/sun/nio/ch/IOUtil.java Wed Apr 17 02:53:02 2013 -0700 +++ b/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/src/share/classes/sun/nio/ch/NativeDispatcher.java Wed Apr 17 02:53:02 2013 -0700 +++ b/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/src/share/classes/sun/nio/ch/SimpleAsynchronousFileChannelImpl.java Wed Apr 17 02:53:02 2013 -0700 +++ b/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/src/share/classes/sun/nio/ch/SocketChannelImpl.java Wed Apr 17 02:53:02 2013 -0700 +++ b/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/src/solaris/classes/sun/nio/ch/FileDispatcherImpl.java Wed Apr 17 02:53:02 2013 -0700 +++ b/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/src/solaris/classes/sun/nio/ch/SinkChannelImpl.java Wed Apr 17 02:53:02 2013 -0700 +++ b/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/src/solaris/classes/sun/nio/ch/SourceChannelImpl.java Wed Apr 17 02:53:02 2013 -0700 +++ b/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/src/solaris/classes/sun/nio/ch/UnixAsynchronousSocketChannelImpl.java Wed Apr 17 02:53:02 2013 -0700 +++ b/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/src/windows/classes/sun/nio/ch/FileDispatcherImpl.java Wed Apr 17 02:53:02 2013 -0700 +++ b/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/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"); + } + } + } +}