RFR: 8320971: Use BufferedInputStream.buf directly when param of implTransferTo() is trusted [v3]

[jmehrens]

On Thu, 30 Nov 2023 11:12:36 GMT, Sergey Tsypanov <stsypanov at openjdk.org> wrote:

>> It looks like we can skip copying of `byte[]` in `BufferedInputStream.implTransferTo()` for `OutputStreams` residing in `java.io`.
>> 
>> See comment by @vlsi in https://github.com/openjdk/jdk/pull/10525/files#diff-e19c508d1bb6ee78697ecca66947c395adda0d9c49a85bf696e677ecbd977af1R612
>
> Sergey Tsypanov has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8320971: Add test

I think it is safe to poison and leak `this.buf` only while we are holding the lock on the BufferedInputStream and while the stream is not required to remember any bytes. However, the current `this.buf` must clear out of bounds data to avoid sharing old data and must be freed from this stream before the lock is released to prevent tampering with the next locking call to BufferedInputStream.


private long implTransferTo(OutputStream out) throws IOException {
    byte[] buffer;
    if (getClass() == BufferedInputStream.class && markpos == -1) {
        int avail = count - pos;
        if (avail > 0) {
            byte[] buffer = getBufIfOpen(false);
            try {
                Arrays.fill(buffer, 0, pos, (byte) 0);
                Arrays.fill(buffer, count, buffer.length, (byte) 0);
                out.write(buffer, pos, count);
                return Math.addExact(avail, getInIfOpen().transferTo(out));
            } catch (ArithmeticException ignore) {
                return Long.MAX_VALUE;
            } finally { //forget current buffer as it was leaked and poisoned
                if (U.compareAndSetReference(this, BUF_OFFSET, buffer, EMPTY)) {
                    pos = count = 0;
                }
            }
        } else {
            return super.transferTo(out);
        }
    }


Thoughts?

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

PR Comment: https://git.openjdk.org/jdk/pull/16879#issuecomment-1837049677