Skip to content

Commit

Permalink
8321718: ProcessTools.executeProcess calls waitFor before logging
Browse files Browse the repository at this point in the history
Reviewed-by: dholmes, jpai
  • Loading branch information
stefank committed Jan 3, 2024
1 parent ba426d6 commit 9ab29f8
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 @@ -108,6 +108,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

3 comments on commit 9ab29f8

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

@GoeLin
Copy link
Member

@GoeLin GoeLin commented on 9ab29f8 Apr 8, 2024

Choose a reason for hiding this comment

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

/backport jdk21u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 9ab29f8 Apr 8, 2024

Choose a reason for hiding this comment

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

@GoeLin the backport was successfully created on the branch backport-GoeLin-9ab29f8d in my personal fork of openjdk/jdk21u-dev. To create a pull request with this backport targeting openjdk/jdk21u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 9ab29f8d from the openjdk/jdk repository.

The commit being backported was authored by Stefan Karlsson on 3 Jan 2024 and was reviewed by David Holmes and Jaikiran Pai.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk21u-dev:

$ git fetch https://github.com/openjdk-bots/jdk21u-dev.git backport-GoeLin-9ab29f8d:backport-GoeLin-9ab29f8d
$ git checkout backport-GoeLin-9ab29f8d
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk21u-dev.git backport-GoeLin-9ab29f8d

Please sign in to comment.