Skip to content

Commit

Permalink
[grid] ensure all states of StartOrDie are handled (#11297)
Browse files Browse the repository at this point in the history
ensure all states of StartOrDie are handled

DriverService.start did not fail, if the execution of the driver process
was never started or the process did not bind the port.
  • Loading branch information
joerg1985 committed Dec 5, 2022
1 parent 21d115e commit 4af3726
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 17 deletions.
5 changes: 5 additions & 0 deletions java/src/org/openqa/selenium/os/CommandLine.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.io.File;
import java.io.OutputStream;
import java.util.Map;
import java.util.concurrent.TimeUnit;

public class CommandLine {

Expand Down Expand Up @@ -100,6 +101,10 @@ public void execute() {
waitFor();
}

public boolean waitForProcessStarted(long duration, TimeUnit unit) {
return process.waitForProcessStarted(duration, unit);
}

public void waitFor() {
try {
process.waitFor();
Expand Down
34 changes: 22 additions & 12 deletions java/src/org/openqa/selenium/os/OsProcess.java
Original file line number Diff line number Diff line change
Expand Up @@ -105,25 +105,32 @@ public void executeAsync() {
}
}

public boolean waitForProcessStarted(long duration, TimeUnit unit) {
return executeWatchdog.waitForProcessStarted(duration, unit);
}

private OutputStream getOutputStream() {
return drainTo == null ? inputOut
: new MultiOutputStream(inputOut, drainTo);
}

public int destroy() {
SeleniumWatchDog watchdog = executeWatchdog;
watchdog.waitForProcessStarted();

// I literally have no idea why we don't try and kill the process nicely on Windows. If you do,
// answers on the back of a postcard to SeleniumHQ, please.
if (!Platform.getCurrent().is(WINDOWS)) {
watchdog.destroyProcess();
watchdog.waitForTerminationAfterDestroy(2, SECONDS);
}
if (watchdog.waitForProcessStarted(2, TimeUnit.MINUTES)) {
// I literally have no idea why we don't try and kill the process nicely on Windows. If you do,
// answers on the back of a postcard to SeleniumHQ, please.
if (!Platform.getCurrent().is(WINDOWS)) {
watchdog.destroyProcess();
watchdog.waitForTerminationAfterDestroy(2, SECONDS);
}

if (isRunning()) {
watchdog.destroyHarder();
watchdog.waitForTerminationAfterDestroy(1, SECONDS);
if (isRunning()) {
watchdog.destroyHarder();
watchdog.waitForTerminationAfterDestroy(1, SECONDS);
}
} else {
log.warning("Tried to destory a process which never started.");
}

// Make a best effort to drain the streams.
Expand Down Expand Up @@ -236,15 +243,18 @@ public void reset() {
starting = true;
}

private void waitForProcessStarted() {
while (starting) {
private boolean waitForProcessStarted(long duration, TimeUnit unit) {
long end = System.currentTimeMillis() + unit.toMillis(duration);
while (starting && System.currentTimeMillis() < end) {
try {
Thread.sleep(50);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new WebDriverException(e);
}
}

return !starting;
}

private void waitForTerminationAfterDestroy(int duration, TimeUnit unit) {
Expand Down
15 changes: 10 additions & 5 deletions java/src/org/openqa/selenium/remote/service/DriverService.java
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,9 @@ public void start() throws IOException {
process.setEnvironmentVariables(environment);
process.copyOutputTo(getOutputStream());
process.executeAsync();
if (!process.waitForProcessStarted(timeout.toMillis(), TimeUnit.MILLISECONDS)) {
throw new WebDriverException("Timed out waiting for driver process to start.");
}

CompletableFuture<StartOrDie> serverStarted = CompletableFuture.supplyAsync(() -> {
waitUntilAvailable();
Expand All @@ -231,13 +234,15 @@ public void start() throws IOException {
try {
StartOrDie status = (StartOrDie) CompletableFuture.anyOf(serverStarted, processFinished)
.get(getTimeout().toMillis() * 2, TimeUnit.MILLISECONDS);
if (status == StartOrDie.SERVER_STARTED) {
processFinished.cancel(true);
} else {
if (status == StartOrDie.PROCESS_DIED) {
switch (status) {
case SERVER_STARTED:
processFinished.cancel(true);
break;
case PROCESS_DIED:
process = null;
throw new WebDriverException("Driver server process died prematurely.");
}
case PROCESS_IS_ACTIVE:
throw new WebDriverException("Timed out waiting for driver server to bind the port.");
}
} catch (ExecutionException | TimeoutException e) {
throw new WebDriverException("Timed out waiting for driver server to start.", e);
Expand Down

0 comments on commit 4af3726

Please sign in to comment.