On Sat, 31 Jul 2021 15:58:07 GMT, Markus KARG <github.com+1701815+mk...@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
