Skip to content

Commit

Permalink
8321718: ProcessTools.executeProcess calls waitFor before logging
Browse files Browse the repository at this point in the history
Backport-of: 9ab29f8dcd1c0092e4251f996bd53c704e87a74a
  • Loading branch information
GoeLin committed Apr 10, 2024
1 parent 38a2173 commit c254aff
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 14 deletions.
8 changes: 8 additions & 0 deletions test/lib/jdk/test/lib/process/OutputAnalyzer.java
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,14 @@ public OutputAnalyzer(String stdout, String stderr, int exitValue)
buffer = OutputBuffer.of(stdout, stderr, exitValue);
}

/**
* Delegate waitFor to the OutputBuffer. This ensures that
* the progress and timestamps are logged correctly.
*/
public void waitFor() {
buffer.waitFor();
}

/**
* Verify that the stdout contents of output buffer is empty
*
Expand Down
51 changes: 38 additions & 13 deletions test/lib/jdk/test/lib/process/OutputBuffer.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ public OutputBufferException(Throwable cause) {
}
}

/**
* Waits for a process to finish, if there is one assocated with
* this OutputBuffer.
*/
public void waitFor();

/**
* Returns the stdout result
*
Expand All @@ -67,6 +73,13 @@ default public List<String> getStdoutAsList() {
* @return stderr result
*/
public String getStderr();


/**
* Returns the exit value
*
* @return exit value
*/
public int getExitValue();

/**
Expand Down Expand Up @@ -136,28 +149,19 @@ private LazyOutputBuffer(Process p, Charset cs) {
}

@Override
public String getStdout() {
return outTask.get();
}

@Override
public String getStderr() {
return errTask.get();
}

@Override
public int getExitValue() {
public void waitFor() {
if (exitValue != null) {
return exitValue;
// Already waited for this process
return;
}

try {
logProgress("Waiting for completion");
boolean aborted = true;
try {
exitValue = p.waitFor();
logProgress("Waiting for completion finished");
aborted = false;
return exitValue;
} finally {
if (aborted) {
logProgress("Waiting for completion FAILED");
Expand All @@ -169,6 +173,22 @@ public int getExitValue() {
}
}

@Override
public String getStdout() {
return outTask.get();
}

@Override
public String getStderr() {
return errTask.get();
}

@Override
public int getExitValue() {
waitFor();
return exitValue;
}

@Override
public long pid() {
return p.pid();
Expand All @@ -186,6 +206,11 @@ private EagerOutputBuffer(String stdout, String stderr, int exitValue) {
this.exitValue = exitValue;
}

@Override
public void waitFor() {
// Nothing to do since this buffer is not associated with a Process.
}

@Override
public String getStdout() {
return stdout;
Expand Down
5 changes: 4 additions & 1 deletion test/lib/jdk/test/lib/process/ProcessTools.java
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,10 @@ public static OutputAnalyzer executeProcess(ProcessBuilder pb, String input,
}

output = new OutputAnalyzer(p, cs);
p.waitFor();

// Wait for the process to finish. Call through the output
// analyzer to get correct logging and timestamps.
output.waitFor();

{ // Dumping the process output to a separate file
var fileName = String.format("pid-%d-output.log", p.pid());
Expand Down

1 comment on commit c254aff

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.