From 7baccd9ee24bcbc2ca5e449d8d3e5b80860cc088 Mon Sep 17 00:00:00 2001 From: Brian Burkhalter Date: Tue, 4 Apr 2023 15:57:56 +0000 Subject: [PATCH] 8303260: (fc) FileChannel::transferFrom should support position > size() Reviewed-by: alanb --- .../java/nio/channels/FileChannel.java | 26 +-- .../classes/sun/nio/ch/FileChannelImpl.java | 2 - .../FileChannel/TransferFromExtend.java | 161 ++++++++++++++++++ 3 files changed, 176 insertions(+), 13 deletions(-) create mode 100644 test/jdk/java/nio/channels/FileChannel/TransferFromExtend.java diff --git a/src/java.base/share/classes/java/nio/channels/FileChannel.java b/src/java.base/share/classes/java/nio/channels/FileChannel.java index 41e9fad7cac..741ad048cb7 100644 --- a/src/java.base/share/classes/java/nio/channels/FileChannel.java +++ b/src/java.base/share/classes/java/nio/channels/FileChannel.java @@ -633,10 +633,10 @@ public abstract class FileChannel * bytes free in its output buffer. * *

This method does not modify this channel's position. If the given - * position is greater than or equal to the file's current size then no bytes are - * transferred. If the target channel has a position then bytes are - * written starting at that position and then the position is incremented - * by the number of bytes written. + * position is greater than or equal to the file's current size then no + * bytes are transferred. If the target channel has a position then bytes + * are written starting at that position and then the position + * is incremented by the number of bytes written. * *

This method is potentially much more efficient than a simple loop * that reads from this channel and writes to the target channel. Many @@ -701,8 +701,10 @@ public abstract class FileChannel * source has reached end-of-stream. * *

This method does not modify this channel's position. If the given - * position is greater than the file's current size then no bytes are - * transferred. If the source channel has a position then bytes are read + * position is greater than or equal to the file's current size then the + * file will be grown to accommodate the new bytes; the values of any bytes + * between the previous end-of-file and the newly-written bytes are + * unspecified. If the source channel has a position then bytes are read * starting at that position and then the position is incremented by the * number of bytes read. * @@ -715,7 +717,7 @@ public abstract class FileChannel * The source channel * * @param position - * The position within the file at which the transfer is to begin; + * The file position at which the transfer is to begin; * must be non-negative * * @param count @@ -761,7 +763,8 @@ public abstract class FileChannel * #read(ByteBuffer)} method, except that bytes are read starting at the * given file position rather than at the channel's current position. This * method does not modify this channel's position. If the given position - * is greater than or equal to the file's current size then no bytes are read.

+ * is greater than or equal to the file's current size then no bytes are + * read.

* * @param dst * The buffer into which bytes are to be transferred @@ -806,9 +809,10 @@ public abstract class FileChannel * #write(ByteBuffer)} method, except that bytes are written starting at * the given file position rather than at the channel's current position. * This method does not modify this channel's position. If the given - * position is greater than or equal to the file's current size then the file will be - * grown to accommodate the new bytes; the values of any bytes between the - * previous end-of-file and the newly-written bytes are unspecified.

+ * position is greater than or equal to the file's current size then the + * file will be grown to accommodate the new bytes; the values of any bytes + * between the previous end-of-file and the newly-written bytes are + * unspecified.

* *

If the file is open in append mode, then * the effect of invoking this method is unspecified. diff --git a/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java b/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java index 4aeafa2b460..72fb1824323 100644 --- a/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java +++ b/src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java @@ -935,8 +935,6 @@ public class FileChannelImpl throw new NonWritableChannelException(); if ((position < 0) || (count < 0)) throw new IllegalArgumentException(); - if (position > size()) - return 0; // System calls supporting fast transfers might not work on files // which advertise zero size such as those in Linux /proc diff --git a/test/jdk/java/nio/channels/FileChannel/TransferFromExtend.java b/test/jdk/java/nio/channels/FileChannel/TransferFromExtend.java new file mode 100644 index 00000000000..538964e2fcd --- /dev/null +++ b/test/jdk/java/nio/channels/FileChannel/TransferFromExtend.java @@ -0,0 +1,161 @@ +/* + * Copyright (c) 2023, 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 8303260 + * @summary Test transferFrom to a position greater than the file size + * @library .. /test/lib + * @build jdk.test.lib.RandomFactory + * @run junit TransferFromExtend + * @key randomness + */ + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.channels.Channels; +import java.nio.channels.FileChannel; +import java.nio.channels.ReadableByteChannel; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Random; +import java.util.stream.Stream; + +import static java.nio.file.StandardOpenOption.*; + +import jdk.test.lib.RandomFactory; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.junit.jupiter.api.Assertions.*; + +public class TransferFromExtend { + private static final Random RND = RandomFactory.getRandom(); + + private static final Path DIR = Path.of(System.getProperty("test.dir", ".")); + + private static Stream paramProvider(int transferSizeMin, + int transferSizeMax) { + List list = new ArrayList(); + int sizeDelta = transferSizeMax - transferSizeMin; + for (int i = 0; i < 10; i++) { + Arguments args = + Arguments.of(RND.nextInt(1024), + transferSizeMin + RND.nextInt(sizeDelta), + 1 + RND.nextInt(2047)); + list.add(args); + } + return list.stream(); + } + + // + // transfer size must be greater than the threshold + // sun.nio.ch.FileChannelImpl::MAPPED_TRANSFER_THRESHOLD (16K) + // for a mapped transfer to be used when direct is unavailable + // + private static Stream fastParamProvider() { + return paramProvider(16*1024 + 1, 500*1024); + } + + private static Stream readingByteChannelParamProvider() { + return paramProvider(1, 64*1024); + } + + /* + * This method tests the optimized path for transferring from a file + * source to a file destination. + */ + @ParameterizedTest + @MethodSource("fastParamProvider") + void fromFileChannel(long initialPosition, int bufSize, long offset) + throws IOException + { + Path file = Files.createTempFile(DIR, "foo", "bar"); + try (FileChannel src = FileChannel.open(file, DELETE_ON_CLOSE, + READ, WRITE)) { + byte[] bytes = new byte[bufSize]; + RND.nextBytes(bytes); + ByteBuffer buf = ByteBuffer.wrap(bytes); + int total = 0; + while (total < bufSize) { + int n = src.write(ByteBuffer.wrap(bytes), 0); + assertTrue(n >= 0, n + " < " + 0); + total += n; + } + testTransferFrom(src, src.size(), initialPosition, offset, bytes); + } + } + + // + // Test the arbitrary source path. This method tests the + // generic path on all platforms. + // + @ParameterizedTest + @MethodSource("readingByteChannelParamProvider") + void fromReadingByteChannel(long initialPosition, int bufSize, long offset) + throws IOException + { + byte[] bytes = new byte[bufSize]; + RND.nextBytes(bytes); + ByteArrayInputStream bais = new ByteArrayInputStream(bytes); + try (ReadableByteChannel src = Channels.newChannel(bais)) { + testTransferFrom(src, bufSize, initialPosition, offset, bytes); + } + } + + /** + * Tests transferring bytes to a FileChannel from a ReadableByteChannel. + * + * @param src the source of the bytes to transfer + * @param count the number of bytes to transfer + * @param initialPos the position of the target channel before the transfer + * @param offset the offset beyong the end of the target channel + * @param bytes the bytes expected to be transferred + */ + private static void testTransferFrom(ReadableByteChannel src, long count, + long initialPos, long offset, + byte[] bytes) + throws IOException + { + Path file = Files.createTempFile(DIR, "foo", "bar"); + try (FileChannel target = FileChannel.open(file, DELETE_ON_CLOSE, + READ, WRITE)) { + target.position(initialPos); + assertEquals(1, target.write(ByteBuffer.wrap(new byte[] {(byte)42}))); + + long position = target.size() + offset; + long transferred = target.transferFrom(src, position, count); + assertTrue(transferred >= 0, "transferFrom returned negative"); + assertFalse(count < transferred, count + " < " + transferred); + ByteBuffer buf = ByteBuffer.allocate((int)transferred); + target.read(buf, position); + assertArrayEquals(Arrays.copyOf(bytes, (int)transferred), + buf.array(), "arrays unequal"); + } + } +}