8220394: bufferedStream does not honor size limit

[Thomas Stüfe]

Thank you David.

..thomas

On Wed 22. May 2019 at 23:13, David Holmes <david.holmes at oracle.com> wrote:

> Hi Thomas,
>
> On 22/05/2019 7:25 pm, Thomas Stüfe wrote:
> > Hi all,
> >
> > third round:
> >
> http://cr.openjdk.java.net/~stuefe/webrevs/8220394-bufferedstream-does-not-honor-size-limit/webrev.02/webrev/
>
> That version is a lot harder to get my head around. I think it is okay.
>
> Thanks,
> David
> -----
>
> > I also added a gtest test case for bufferedStream. It tests the static
> > variant, the dynamic variant without truncation and with truncation. The
> > last test, testing with truncation, is commented out - to run this test,
> > you need to manually enable it first. This is because it uses a lot of
> > memory (100M) in release and in debug it would crash as expected.
> >
> > Cheers, Thomas
> >
> >
> >
> >
> > On Tue, May 21, 2019 at 4:18 PM Thomas Stüfe <thomas.stuefe at gmail.com
> > <mailto:thomas.stuefe at gmail.com>> wrote:
> >
> >     Hi,
> >
> >     new webrev with the following changes:
> >
> >     - reworked comment to make it more concise and clear
> >     - the cap where we assert now is guaranteed to be larger than the
> >     buffer maximum.
> >
> >
> http://cr.openjdk.java.net/~stuefe/webrevs/8220394-bufferedstream-does-not-honor-size-limit/webrev.01/webrev/
> >
> >     Thanks, Thomas
> >
> >
> >
> >     On Tue, May 14, 2019 at 12:25 PM Thomas Stüfe
> >     <thomas.stuefe at gmail.com <mailto:thomas.stuefe at gmail.com>> wrote:
> >
> >             Hi all,
> >
> >
> >         Bug: https://bugs.openjdk.java.net/browse/JDK-8220394
> >         cr:
> >
> http://cr.openjdk.java.net/~stuefe/webrevs/8220394-bufferedstream-does-not-honor-size-limit/webrev.00/webrev/
> >
> >         I redid the patch for this in a minimal form. The first version
> >         I posted in March but after consideration I found it too
> invasive.
> >
> >         Short story: bufferedStream is misused as a
> >         outputStream-with-dynamic-buffer in a number of places - misused
> >         since we have stringStream for that.
> >
> >         It has something looking like a maximum buffer size cap but that
> >         is actually a flush-trigger for child classes of bufferesStream.
> >         bufferedStream::flush itself is a noop.
> >
> >         That means that printing to this stream may cause high memory
> >         footprint or native OOM since the upper limit is not honored. In
> >         runaway printing coding this can tear down the process.
> >
> >         This patch is a stopgap - when we reach the buffer limit - but
> >         not below 100M - we will assert(debug) or truncate (release).
> >
> >         I am careful here since I do not know if there are situations
> >         where more than buffer-limit bytes are written today and
> >         suddenly enforcing this limit now would cause errors. I think
> >         100M is safe to be considered "too much".
> >
> >         The real correct solution should be that all callers use
> >         stringStream and handle truncation. This patch is small and
> >         could be, if necessary, ported down to older releases easily.
> >
> >         Thanks, Thomas
> >
> >
>