On Thu, 30 Nov 2023 11:12:36 GMT, Sergey Tsypanov <stsypa...@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

Reply via email to