On Tue, 19 Apr 2022 22:37:21 GMT, jmehrens <d...@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
