8220394: bufferedStream does not honor size limit

[Thomas Stüfe]

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>
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
>
>
>