RFR: JDK-8319307: DCmds should not assert on truncation and should report truncation [v2]

[Kevin Walls]

On Sat, 4 Nov 2023 06:30:29 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> `bufferedStream` was originally intended to provide intermediate output buffering; the buffer was supposed to drain upon flushing. However, in many cases, `flush()` is a no-op, so the buffer never gets drained.
>> 
>> To prevent infinitely raising buffer sizes for non-flushing `bufferedStream`, [JDK-8220394](https://bugs.openjdk.org/browse/JDK-8220394) introduced the notion of a "maximum reasonable cap". Upon reaching this threshold, we assert in debug VMs since we assumed this to be a condition worth analyzing. In release VMs, we silently truncate.
>> 
>> But DCmds - one of the primary users of `bufferedStream` - can reach the maximum cap under normal conditions; one example would be printing the list of dynamic libraries on Linux (just prints the process memory map) - this can get very large. Similarly, NMT detail reports and VM.info output can get just as large.
>> 
>> Therefore, neither asserting nor silent truncation is optimal. Instead, we should truncate the output, print a visible truncation marker, and - if possible - interrupt the printing.
>> 
>> ---
>> 
>> The patch is minimally invasive to simplify review. Like most stream classes, `bufferedStream` would benefit from an overhaul, but I'd like to leave that to a future RFE.
>> 
>> Testing: Tested manually with a number of commands with artificially increased output size. GHAs (Windows test errors unrelated).
>
> Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into JDK-8319307-DCmds-should-not-assert-on-truncation-and-should-report-truncation
>  - fix mac builds
>  - JDK-8319307-DCmds-should-not-assert-on-truncation-and-should-report-truncation

Hi,
What jcmds are hitting the limit?  Dynamic library list on Linux is mentioned in the bug, so is that a "jcmd PID VM.dynlibs" command which is printing 100MB of output? (/proc/pid/maps is >100MB)

Is the problem that the buffer has nowhere to drain to at the point the diagnostic command is running?   AttachListener executes the whole diagnostic command into a buffer, then calls op->complete(...); which prints the result using e.g. LinuxAttachListener::write_fully().

If this no longer works, maybe it's time to use a version of bufferedStream which has knowledge of where output is going.  The attachlistener creates the buffer and tells it the destination (the attach listener's socket) so it can actually write it out and clear the buffer when necessary.  The AttachListener's call to op->complete would just flush() the buffer.

If it's only os::print_dll_info(outputStream *st) which is known to fill the buffer, then that one function could limit its own output until there is such a better solution.

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

PR Comment: https://git.openjdk.org/jdk/pull/16474#issuecomment-1827609414