On Fri, 12 May 2023 12:47:28 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> Can I please get a review of this test only change which addresses the issue > noted in https://bugs.openjdk.org/browse/JDK-8307403? > > When we recently did a change in https://bugs.openjdk.org/browse/JDK-8299748, > there was a oversight that the `deflate.deflate(tempBuffer)` could > potentially end up taking in a zero sized array. That would then mean the > loop in which this `deflate` happens, would never exit leading to the test > timing out. > I've added additional details as a comment to the JBS issue > https://bugs.openjdk.org/browse/JDK-8307403?focusedCommentId=14581202&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14581202. > > The commit in this PR uses a fixed size temporary buffer, unrelated to the > input length, to get past this problem. The use of fixed size here will not > re-introduce the original issue which > https://bugs.openjdk.org/browse/JDK-8299748 fixed, because unlike previously, > this is just a temporary size and we continue to deflate the content into a > dynamically growing `ByteArrayOutputStream` till the deflater is "finished". > > Additionally I've added a log message to help debug any future issues in this > call. Looks good except the comment below. test/jdk/java/util/zip/DeInflate.java line 146: > 144: def.finish(); > 145: > 146: try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) { I don't think this is really required because the `ByteArrayOutputStream` will grow dynamically anyway, even if initialized to zero but for all other cases `len` is probably a better estimation than the default size of `32`. But I leave it up to you if you want to change it back. ------------- Marked as reviewed by simonis (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/13955#pullrequestreview-1426788585 PR Review Comment: https://git.openjdk.org/jdk/pull/13955#discussion_r1193983220
