RFR: 8233725: ProcessTools.startProcess() has output issues when using an OutputAnalyzer at the same time

Serguei Spitsyn sspitsyn at openjdk.org
Wed Apr 26 09:49:54 UTC 2023


On Fri, 21 Apr 2023 21:43:39 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:

> ProcessTools.startProcess() creates process and read it's output error streams. So the any other using of corresponding Process.getInputStream() and Process.getErrorStream() doesn't get process streams.
> 
> This fix preserve process streams content and allow to read reuse the date. The ByteArrayOutputStream is used as a buffer. 
> It stores all process output, never trying to clean date which has been read. 
> 
> The regression test has been provided with issue.
> 
> I closed previous PR https://github.com/openjdk/jdk/pull/13560 by mistake instead of updating it.
> 
> I run all tests to ensure that no failures are introduced.

Looks good. Posted a few minor comments.
Thanks,
Serguei

test/lib/jdk/test/lib/process/ProcessTools.java line 170:

> 168:         }
> 169: 
> 170:         synchronized  int readNext() {

An extra space after synchronized keyword.

test/lib/jdk/test/lib/process/ProcessTools.java line 172:

> 170:         synchronized  int readNext() {
> 171:             if (current > count) {
> 172:                 throw new RuntimeException("Shouldn't ever happen.  start: " + current + " count: " + count + " buffer: " + this);

This line is too long.

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

Marked as reviewed by sspitsyn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13594#pullrequestreview-1401596190
PR Review Comment: https://git.openjdk.org/jdk/pull/13594#discussion_r1177621994
PR Review Comment: https://git.openjdk.org/jdk/pull/13594#discussion_r1177622644


More information about the serviceability-dev mailing list