RFR: 8284638: store skip buffers in InputStream Object

[liach]

On Fri, 8 Oct 2021 21:19:36 GMT, XenoAmess <duke at openjdk.java.net> wrote:

> @jmehrens what about this then?
> I think it safe now(actually this mechanism is learned from Reader)

You should really post your changes to jdk-dev at openjdk.java.net before submitting these so you can know if a change is potentially good or bad. Since jdk pull requests require issue numbers, you can ask people on the list if this is a good idea; if it is, someone on the list may create an issue for you, which probably means your suggestion is valid, and only then you should send a pull request.

You need to get a ticket so this pr may get merged. probably through suggesting enhancement on https://bugs.java.com (recommended, takes time but guarantees response) or asking on the mailing list

src/java.base/share/classes/java/io/InputStream.java line 550:

> 548: 
> 549:         if ((skipBuffer == null) || (skipBuffer.length < size)) {
> 550:             skipBuffer = new byte[size];

In the `Reader` class, `skipBuffer` access is thread-safe as it's behind a synchronization on the `lock`. This one isn't and is subject to weird java memory models: in extreme cases, if the `skipBuffer` is set to non-null on another thread, it may be non-null on first field read and null on second field read.

Hence you should write code like this instead:

        byte[] skipBuffer = this.skipBuffer;
        if ((skipBuffer == null) || (skipBuffer.length < size)) {
            this.skipBuffer = skipBuffer = new byte[size];
        }

so you only read the field once and get consistent results.

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

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