Skip to content

Commit

Permalink
[java] close the streams after I/O failure #13096
Browse files Browse the repository at this point in the history
  • Loading branch information
joerg1985 committed Nov 6, 2023
1 parent afa349a commit e4d31f2
Showing 1 changed file with 11 additions and 16 deletions.
27 changes: 11 additions & 16 deletions java/src/org/openqa/selenium/os/ExternalProcess.java
Expand Up @@ -193,29 +193,26 @@ public ExternalProcess start() throws UncheckedIOException {
throw new UncheckedIOException(ex);
}

CircularOutputStream circular;
try {
circular = new CircularOutputStream(bufferSize);
CircularOutputStream circular = new CircularOutputStream(bufferSize);

new Thread(
() -> {
InputStream input = process.getInputStream();
// use the CircularOutputStream as mandatory, we know it will never raise a
// IOException
OutputStream output = new MultiOutputStream(circular, copyOutputTo);
while (process.isAlive()) {
try {
// we must read the output to ensure the process will not lock up
input.transferTo(output);
} catch (IOException ex) {
LOG.log(
Level.WARNING,
"failed to copy the output of process " + process.pid(),
ex);
}
try (InputStream input = process.getInputStream();
OutputStream output = new MultiOutputStream(circular, copyOutputTo)) {
// we must read the output to ensure the process will not lock up
input.transferTo(output);
} catch (IOException ex) {
LOG.log(
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);
} catch (Throwable t) {
// ensure we do not leak a process in case of failures
try {
Expand All @@ -225,8 +222,6 @@ public ExternalProcess start() throws UncheckedIOException {
}
throw t;
}

return new ExternalProcess(process, circular);
}
}

Expand Down

1 comment on commit e4d31f2

@titusfortner
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this broke our CI.

Please sign in to comment.