Skip to content

Commit 7ddfad6

Browse files
joerg1985joerg1985
authored
[java] use the java process builder to run external processes (#12898)
1 parent 36108e7 commit 7ddfad6

File tree

9 files changed

+497
-56
lines changed

9 files changed

+497
-56
lines changed

‎java/src/org/openqa/selenium/manager/SeleniumManager.java

+9-9
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.nio.file.Paths;
2929
import java.nio.file.SimpleFileVisitor;
3030
import java.nio.file.attribute.BasicFileAttributes;
31+
import java.time.Duration;
3132
import java.util.ArrayList;
3233
import java.util.Arrays;
3334
import java.util.List;
@@ -43,7 +44,7 @@
4344
import org.openqa.selenium.json.Json;
4445
import org.openqa.selenium.json.JsonException;
4546
import org.openqa.selenium.manager.SeleniumManagerOutput.Result;
46-
import org.openqa.selenium.os.CommandLine;
47+
import org.openqa.selenium.os.ExternalProcess;
4748

4849
/**
4950
* This implementation is still in beta, and may change.
@@ -112,15 +113,14 @@ private static Result runCommand(Path binary, List<String> arguments) {
112113
String output;
113114
int code;
114115
try {
115-
CommandLine command =
116-
new CommandLine(binary.toAbsolutePath().toString(), arguments.toArray(new String[0]));
117-
command.executeAsync();
118-
command.waitFor();
119-
if (command.isRunning()) {
120-
LOG.warning("Selenium Manager did not exit");
116+
ExternalProcess process =
117+
ExternalProcess.builder().command(binary.toAbsolutePath().toString(), arguments).start();
118+
if (!process.waitFor(Duration.ofHours(1))) {
119+
LOG.warning("Selenium Manager did not exit, shutting it down");
120+
process.shutdown();
121121
}
122-
code = command.getExitCode();
123-
output = command.getStdOut();
122+
code = process.exitValue();
123+
output = process.getOutput();
124124
} catch (Exception e) {
125125
throw new WebDriverException("Failed to run command: " + arguments, e);
126126
}

‎java/src/org/openqa/selenium/os/CommandLine.java

+1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.openqa.selenium.Platform;
2828
import org.openqa.selenium.WebDriverException;
2929

30+
@Deprecated
3031
public class CommandLine {
3132

3233
private final OsProcess process;

‎java/src/org/openqa/selenium/os/ExternalProcess.java

+286
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,286 @@
1+
// Licensed to the Software Freedom Conservancy (SFC) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The SFC licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.openqa.selenium.os;
19+
20+
import static java.util.concurrent.TimeUnit.SECONDS;
21+
22+
import java.io.File;
23+
import java.io.IOException;
24+
import java.io.InputStream;
25+
import java.io.OutputStream;
26+
import java.io.UncheckedIOException;
27+
import java.time.Duration;
28+
import java.util.ArrayList;
29+
import java.util.Collections;
30+
import java.util.List;
31+
import java.util.Map;
32+
import java.util.concurrent.TimeUnit;
33+
import java.util.logging.Level;
34+
import java.util.logging.Logger;
35+
import org.openqa.selenium.internal.Require;
36+
import org.openqa.selenium.io.CircularOutputStream;
37+
import org.openqa.selenium.io.MultiOutputStream;
38+
39+
public class ExternalProcess {
40+
private static final Logger LOG = Logger.getLogger(ExternalProcess.class.getName());
41+
42+
public static class Builder {
43+
44+
private ProcessBuilder builder;
45+
private OutputStream copyOutputTo;
46+
private int bufferSize = 32768;
47+
48+
Builder() {
49+
this.builder = new ProcessBuilder();
50+
}
51+
52+
/**
53+
* Set the executable command to start the process, this consists of the executable and the
54+
* arguments.
55+
*
56+
* @param executable the executable to build the command
57+
* @param arguments the arguments to build the command
58+
* @return this instance to continue building
59+
*/
60+
public Builder command(String executable, List<String> arguments) {
61+
List<String> command = new ArrayList<>(arguments.size() + 1);
62+
command.add(executable);
63+
command.addAll(arguments);
64+
builder.command(command);
65+
66+
return this;
67+
}
68+
69+
/**
70+
* Set the executable command to start the process, this consists of the executable and the
71+
* arguments.
72+
*
73+
* @param command the executable, followed by the arguments
74+
* @return this instance to continue building
75+
*/
76+
public Builder command(List<String> command) {
77+
builder.command(command);
78+
79+
return this;
80+
}
81+
82+
/**
83+
* Set the executable command to start the process, this consists of the executable and the
84+
* arguments.
85+
*
86+
* @param command the executable, followed by the arguments
87+
* @return this instance to continue building
88+
*/
89+
public Builder command(String... command) {
90+
builder.command(command);
91+
92+
return this;
93+
}
94+
95+
/**
96+
* Get the executable command to start the process, this consists of the binary and the
97+
* arguments.
98+
*
99+
* @return an editable list, changes to it will update the command executed.
100+
*/
101+
public List<String> command() {
102+
return Collections.unmodifiableList(builder.command());
103+
}
104+
105+
/**
106+
* Set one environment variable of the process to start, will replace the old value if exists.
107+
*
108+
* @return this instance to continue building
109+
*/
110+
public Builder environment(String name, String value) {
111+
Require.argument("name", name).nonNull();
112+
Require.argument("value", value).nonNull();
113+
114+
builder.environment().put(name, value);
115+
116+
return this;
117+
}
118+
119+
/**
120+
* Get the environment variables of the process to start.
121+
*
122+
* @return an editable map, changes to it will update the environment variables of the command
123+
* executed.
124+
*/
125+
public Map<String, String> environment() {
126+
return builder.environment();
127+
}
128+
129+
/**
130+
* Get the working directory of the process to start, maybe null.
131+
*
132+
* @return the working directory
133+
*/
134+
public File directory() {
135+
return builder.directory();
136+
}
137+
138+
/**
139+
* Set the working directory of the process to start.
140+
*
141+
* @param directory the path to the directory
142+
* @return this instance to continue building
143+
*/
144+
public Builder directory(String directory) {
145+
return directory(new File(directory));
146+
}
147+
148+
/**
149+
* Set the working directory of the process to start.
150+
*
151+
* @param directory the path to the directory
152+
* @return this instance to continue building
153+
*/
154+
public Builder directory(File directory) {
155+
builder.directory(directory);
156+
157+
return this;
158+
}
159+
160+
/**
161+
* Where to copy the combined stdout and stderr output to, {@code OsProcess#getOutput} is still
162+
* working when called.
163+
*
164+
* @param stream where to copy the combined output to
165+
* @return this instance to continue building
166+
*/
167+
public Builder copyOutputTo(OutputStream stream) {
168+
copyOutputTo = stream;
169+
170+
return this;
171+
}
172+
173+
/**
174+
* The number of bytes to buffer for {@code OsProcess#getOutput} calls.
175+
*
176+
* @param toKeep the number of bytes, default is 4096
177+
* @return this instance to continue building
178+
*/
179+
public Builder bufferSize(int toKeep) {
180+
bufferSize = toKeep;
181+
182+
return this;
183+
}
184+
185+
public ExternalProcess start() throws UncheckedIOException {
186+
// redirect the stderr to stdout
187+
builder.redirectErrorStream(true);
188+
189+
Process process;
190+
try {
191+
process = builder.start();
192+
} catch (IOException ex) {
193+
throw new UncheckedIOException(ex);
194+
}
195+
196+
CircularOutputStream circular;
197+
try {
198+
circular = new CircularOutputStream(bufferSize);
199+
200+
new Thread(
201+
() -> {
202+
InputStream input = process.getInputStream();
203+
// use the CircularOutputStream as mandatory, we know it will never raise a
204+
// IOException
205+
OutputStream output = new MultiOutputStream(circular, copyOutputTo);
206+
while (process.isAlive()) {
207+
try {
208+
// we must read the output to ensure the process will not lock up
209+
input.transferTo(output);
210+
} catch (IOException ex) {
211+
LOG.log(
212+
Level.WARNING,
213+
"failed to copy the output of process " + process.pid(),
214+
ex);
215+
}
216+
}
217+
})
218+
.start();
219+
} catch (Throwable t) {
220+
// ensure we do not leak a process in case of failures
221+
try {
222+
process.destroyForcibly();
223+
} catch (Throwable t2) {
224+
t.addSuppressed(t2);
225+
}
226+
throw t;
227+
}
228+
229+
return new ExternalProcess(process, circular);
230+
}
231+
}
232+
233+
public static Builder builder() {
234+
return new Builder();
235+
}
236+
237+
private final Process process;
238+
private final CircularOutputStream outputStream;
239+
240+
public ExternalProcess(Process process, CircularOutputStream outputStream) {
241+
this.process = process;
242+
this.outputStream = outputStream;
243+
}
244+
245+
/**
246+
* The last N bytes of the combined stdout and stderr as String, the value of N is set while
247+
* building the OsProcess.
248+
*
249+
* @return stdout and stderr as String in Charset.defaultCharset() encoding
250+
*/
251+
public String getOutput() {
252+
return outputStream.toString();
253+
}
254+
255+
public boolean isAlive() {
256+
return process.isAlive();
257+
}
258+
259+
public boolean waitFor(Duration duration) throws InterruptedException {
260+
return process.waitFor(duration.toMillis(), TimeUnit.MILLISECONDS);
261+
}
262+
263+
public int exitValue() {
264+
return process.exitValue();
265+
}
266+
267+
/**
268+
* Initiate a normal shutdown of the process or kills it when the process is alive after 4
269+
* seconds.
270+
*/
271+
public void shutdown() {
272+
if (process.supportsNormalTermination()) {
273+
process.destroy();
274+
275+
try {
276+
if (process.waitFor(4, SECONDS)) {
277+
return;
278+
}
279+
} catch (InterruptedException ex) {
280+
Thread.interrupted();
281+
}
282+
}
283+
284+
process.destroyForcibly();
285+
}
286+
}

‎java/src/org/openqa/selenium/os/OsProcess.java

+1
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.openqa.selenium.io.CircularOutputStream;
4545
import org.openqa.selenium.io.MultiOutputStream;
4646

47+
@Deprecated
4748
class OsProcess {
4849
private static final Logger LOG = Logger.getLogger(OsProcess.class.getName());
4950

‎java/src/org/openqa/selenium/remote/service/DriverCommandExecutor.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,11 @@ public Response execute(Command command) throws IOException {
145145
CompletableFuture<Response> processFinished =
146146
CompletableFuture.supplyAsync(
147147
() -> {
148-
service.process.waitFor(service.getTimeout().toMillis());
148+
try {
149+
service.process.waitFor(service.getTimeout());
150+
} catch (InterruptedException ex) {
151+
Thread.currentThread().interrupt();
152+
}
149153
return null;
150154
},
151155
executorService);

0 commit comments

Comments
 (0)