RFR: 8265891: ChannelInputStream.transferTo() uses FileChannel.transferTo()
Markus KARG
github.com+1701815+mkarg at openjdk.java.net
Sat Aug 21 14:46:22 UTC 2021
On Thu, 19 Aug 2021 18:25:39 GMT, Alan Bateman <alanb at openjdk.org> wrote:
>>> I had hoped that ThrowingLongSupplier would go away. If you keep your 2-arg transfer method then we should be able to implementation transferTo very simply, something like
>>>
>>> ```
>>> public long transferTo(OutputStream out) throws IOException {
>>> if (ch instanceof FileChannel fc
>>> && out instanceof ChannelOutputStream cos) {
>>> WritableByteChannel target = cos.channel();
>>> if (target instanceof SelectableChannel sc) {
>>> synchronized (sc.blockingLock()) {
>>> if (!sc.isBlocking())
>>> throw new IllegalBlockingModeException();
>>> return transfer(fc, target);
>>> }
>>> }
>>> return transfer(fc, target);
>>> }
>>> return super.transferTo(out);
>>> }
>>> ```
>>
>> This is not true. It will only check the blocking mode for the target. But technically it is possible that someone writes an implementation of FileChannel which implements SelectableChannel. In that case it is needed to check the blocking mode on the source, too. So either we (a) ignore this possibility, (b) write a very complex code which calls transfer(fc, target) in FOUR code paths not just two, or (c) we keep the ThrowingLongSupplier as an elegant way to perform the 2x2 matrix.
>
>> This is not true. It will only check the blocking mode for the target. But technically it is possible that someone writes an implementation of FileChannel which implements SelectableChannel. In that case it is needed to check the blocking mode on the source, too. So either we (a) ignore this possibility, (b) write a very complex code which calls transfer(fc, target) in FOUR code paths not just two, or (c) we keep the ThrowingLongSupplier as an elegant way to perform the 2x2 matrix.
>
> SelectableChannel is an abstract class, not an interface so you can't extend both FileChannel and SelectableChannel. So when the source channel is a FileChannel then it should only be concerned with whether the target channel is a SelectableChannel or not. I think we can keep it very simple, along the lines of the method I pasted in above.
>
> If you choose to include the case where the target is a SelectableChannel (and I have no objection to doing that) then the test needs to exercise that code. This means the test needs an input stream created on a source file/FileChannel, and an output stream created an channel that is WritableByteChannel & SelectableChannel. A Pipe.SinkChannel or a connected SocketChannel will do fine. If you choose to leave out selectable channels for the first PR then file -> file is okay, which I think is what your current test is doing.
@AlanBateman I have pushed another PR https://github.com/openjdk/jdk/pull/5209 that handles only the case FileChannel->FileChannel, so we will have some progress there and once *that* one is merged I will come back to the FileChannel->SelectableChannel case later.
-------------
PR: https://git.openjdk.java.net/jdk/pull/5179
More information about the nio-dev
mailing list