RFR: 8265891: (ch) InputStream returned by Channels.newInputStream should override transferTo [v6]

[Alan Bateman]

On Sat, 31 Jul 2021 15:58:07 GMT, Markus KARG <github.com+1701815+mkarg at openjdk.org> wrote:

>>> Is this loop correct for the case that the channel gets truncated? In that case transferTo will return 0 as no bytes will be transferred and I'm concerned this code will go into an infinite loop.
>> 
>> Good catch, indeed missed this particular situation!
>> 
>> The modified code found in https://github.com/openjdk/jdk/pull/4263/commits/4b501b205c6f1c48bbc82d7a154aed3fc41b1a20 should be safe from infinite loops, as it checks the actual file length in each iteration (even if this costs us one more `synchronized` per iteration).
>> 
>> This will also improve the situation with concurrent *extension* of the file as outlined in:
>> 
>>>If src is extended at an inconvenient point in time, for example between the return from a call to src.transferTo() that makes bytesWritten < n false and the evaluation of that terminating condition, then it appears that not all the content of src will be transferred to dest. I am not however sure that this violates any contract but it could be a behavioral change from the existing implementation.
>> 
>> As `size()` is called in each iteration, it will pick up the appendend bytes. Still the situation exists that some bytes are *replaced* in the source file -- but that is actually out-of-scope of this PR...! ;-)
>
>> Also can you can check that IllegalBlockingModeException is thrown for the case ch is a SelectableChannel configured non-blocking and the destination is a FileChannel?
> 
> Done in https://github.com/openjdk/jdk/pull/4263/commits/8e2889fd6138d8aa8e308a1cd2aea3546a7c43a7, but honestly I'd kindly like to ask for a brief explanation why this has to be done. What actual bad effect would happen if I do not throw?

> The modified code found in [4b501b2](https://github.com/openjdk/jdk/commit/4b501b205c6f1c48bbc82d7a154aed3fc41b1a20) should be safe from infinite loops, as it checks the actual file length in each iteration (even if this costs us one more `synchronized` per iteration).

I need to look at it closely but I suspect this introduces a potential overflow. Also if output stream is backed by a SocketChannel configured non-blocking then FC::transferTo may return 0 so I assume there is a potential infinite loop there too. I suspect the eventually patch will need have to make use of the blockingLock to prevent the underlying channels from being changed to non-blocking during the transfer.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4263