On Sat, 31 Jul 2021 13:12:54 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. >> >> 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? >> >> Just on naming, the existing channel implementations use "dst" for the >> destination and "wbc" (not "oc") for writable byte channels. Just mentioning >> it so that the new code can be kept consistent where possible. > >> Just on naming, the existing channel implementations use "dst" for the >> destination and "wbc" (not "oc") for writable byte channels. Just mentioning >> it so that the new code can be kept consistent where possible. > > I have renamed `dest` to `dst` and `oc` to `wbc` in > https://github.com/openjdk/jdk/pull/4263/commits/f91d5422c41e25410a81aad54a45b2d0e171305a. > 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...! ;-) ------------- PR: https://git.openjdk.java.net/jdk/pull/4263
