Skip to content

Commit

Permalink
[java] ensure the complete output is read #13091
Browse files Browse the repository at this point in the history
  • Loading branch information
joerg1985 committed Nov 9, 2023
1 parent 1bccc05 commit d1787a9
Showing 1 changed file with 22 additions and 6 deletions.
28 changes: 22 additions & 6 deletions java/src/org/openqa/selenium/os/ExternalProcess.java
Expand Up @@ -196,7 +196,8 @@ public ExternalProcess start() throws UncheckedIOException {
try {
CircularOutputStream circular = new CircularOutputStream(bufferSize);

new Thread(
Thread worker =
new Thread(
() -> {
// copyOutputTo might be system.out or system.err, do not to close
OutputStream output = new MultiOutputStream(circular, copyOutputTo);
Expand All @@ -212,10 +213,11 @@ public ExternalProcess start() throws UncheckedIOException {
Level.WARNING, "failed to copy the output of process " + process.pid(), ex);
}
LOG.log(Level.FINE, "completed to copy the output of process " + process.pid());
})
.start();
});

return new ExternalProcess(process, circular);
worker.start();

return new ExternalProcess(process, circular, worker);
} catch (Throwable t) {
// ensure we do not leak a process in case of failures
try {
Expand All @@ -234,10 +236,12 @@ public static Builder builder() {

private final Process process;
private final CircularOutputStream outputStream;
private final Thread worker;

public ExternalProcess(Process process, CircularOutputStream outputStream) {
public ExternalProcess(Process process, CircularOutputStream outputStream, Thread worker) {
this.process = process;
this.outputStream = outputStream;
this.worker = worker;
}

/**
Expand All @@ -255,7 +259,13 @@ public boolean isAlive() {
}

public boolean waitFor(Duration duration) throws InterruptedException {
return process.waitFor(duration.toMillis(), TimeUnit.MILLISECONDS);
boolean exited = process.waitFor(duration.toMillis(), TimeUnit.MILLISECONDS);

if (exited) {
worker.join();
}

return exited;
}

public int exitValue() {
Expand All @@ -282,6 +292,7 @@ public void shutdown(Duration timeout) {

try {
if (process.waitFor(timeout.toMillis(), MILLISECONDS)) {
worker.join();
return;
}
} catch (InterruptedException ex) {
Expand All @@ -290,5 +301,10 @@ public void shutdown(Duration timeout) {
}

process.destroyForcibly();
try {
worker.join();

This comment has been minimized.

Copy link
@titusfortner

titusfortner Jan 3, 2024

Member

@joerg1985 / @diemol this line of code is causing things to hang on Windows. It eventually times out after 30 minutes. No additional info is given — https://github.com/SeleniumHQ/selenium/actions/runs/7391937211/job/20112870732#step:15:267
But I've debugged things to that line of code.

This comment has been minimized.

Copy link
@titusfortner

titusfortner Jan 3, 2024

Member

The server process has terminated, the thread join just hands

} catch (InterruptedException ex) {
Thread.interrupted();
}
}
}

0 comments on commit d1787a9

Please sign in to comment.