RFR: 8284638: store skip buffers in InputStream Object [v7]

XenoAmess duke at openjdk.java.net
Wed Apr 20 16:39:29 UTC 2022


On Tue, 19 Apr 2022 22:37:21 GMT, jmehrens <duke at openjdk.java.net> wrote:

>>> > @jmehrens what about this then? I think it safe now(actually this mechanism is learned from Reader)
>>> 
>>> Reader uses a lock object and it appears that InputStream locks on this (per make/reset) I would assume now that you have some object member fields you need to hold some lock while accessing those. Even though single thread access would be the expected case.
>> 
>> no, we use other way, but yes for we take care of thread safety.
>> 
>>> Not related but, it looks like the static buffer issue has come up a few times. Maybe time to add a test to: https://github.com/openjdk/jdk/blob/master/test/jdk/java/io/InputStream/Skip.java that would fail if the skip buffer is static?
>> 
>> I think it is worthy to add some comments, but a test for it sound a little bit extreme.
>> 
>>> Not really the primary issue but, it seems questionable to me that we don't have to fill the skip buffer with zeros after the last read. That way any sensitive information that was skipped would also be forgotten. Matching with Reader seems more important for now.
>> 
>> Don't think it necessary... I think making it cannot touched by other object (with security manager on) is enough.
>
>> no, we use other way, but yes for we take care of thread safety.
> 
> Fair enough.
> 
>> Don't think it necessary... I think making it cannot touched by other object (with security manager on) is enough.
> 
> I was thinking more for heapdumps due to extending the life of the skip buffer in this patch.  At least we don't have to waste more cycles.
> 
> Oh I forgot last time but, it looks like MIN_SKIP_BUFFER_SIZE is not used.  Unless I'm missing something I assume that is a bug.

@jmehrens comments about not making it static added.

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

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


More information about the core-libs-dev mailing list