refactor: Migrate to new Selenium API for process management (appium#2054)

Author: valfirst

src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java

@@ -17,10 +17,9 @@
 package io.appium.java_client.service.local;
 
 import com.google.common.annotations.VisibleForTesting;
-import io.appium.java_client.internal.ReflectionHelpers;
 import lombok.SneakyThrows;
 import org.openqa.selenium.net.UrlChecker;
-import org.openqa.selenium.os.CommandLine;
+import org.openqa.selenium.os.ExternalProcess;
 import org.openqa.selenium.remote.service.DriverService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -49,6 +48,7 @@
 import static io.appium.java_client.service.local.AppiumServiceBuilder.BROADCAST_IP4_ADDRESS;
 import static io.appium.java_client.service.local.AppiumServiceBuilder.BROADCAST_IP6_ADDRESS;
 import static java.util.Objects.requireNonNull;
+import static java.util.Optional.ofNullable;
 import static org.slf4j.event.Level.DEBUG;
 import static org.slf4j.event.Level.INFO;
 
@@ -70,7 +70,7 @@ public final class AppiumDriverLocalService extends DriverService {
  private final URL url;
  private String basePath;
 
- private CommandLine process = null;
+ private ExternalProcess process = null;
 
  AppiumDriverLocalService(String ipAddress, File nodeJSExec,
  int nodeJSPort, Duration startupTimeout,
@@ -126,7 +126,7 @@ public URL getUrl() {
  public boolean isRunning() {
  lock.lock();
  try {
- if (process == null || !process.isRunning()) {
+ if (process == null || !process.isAlive()) {
  return false;
  }
 
@@ -172,17 +172,15 @@ public void start() throws AppiumServerHasNotBeenStartedLocallyException {
  }
 
  try {
- process = new CommandLine(
- this.nodeJSExec.getCanonicalPath(),
- nodeJSArgs.toArray(new String[]{})
- );
- process.setEnvironmentVariables(nodeJSEnvironment);
- process.copyOutputTo(stream);
- process.executeAsync();
+ ExternalProcess.Builder processBuilder = ExternalProcess.builder()
+ .command(this.nodeJSExec.getCanonicalPath(), nodeJSArgs)
+ .copyOutputTo(stream);
+ nodeJSEnvironment.forEach(processBuilder::environment);
+ process = processBuilder.start();
  ping(startupTimeout);
  } catch (Exception e) {
- final Optional<String> output = Optional.ofNullable(process)
- .map(CommandLine::getStdOut)
+ final Optional<String> output = ofNullable(process)
+ .map(ExternalProcess::getOutput)
  .filter(o -> !isNullOrEmpty(o));
  destroyProcess();
  List<String> errorLines = new ArrayList<>();
@@ -227,47 +225,16 @@ public void stop() {
  }
  }
 
- /**
- * Destroys the service if it is running.
- *
- * @param timeout The maximum time to wait before the process will be force-killed.
- * @return The exit code of the process or zero if the process was not running.
- */
- private int destroyProcess(Duration timeout) {
- if (process == null || !process.isRunning()) {
- return 0;
- }
-
- // This all magic is necessary, because Selenium does not publicly expose
- // process killing timeouts. By default a process is killed forcibly if
- // it does not exit after two seconds, which is in most cases not enough for
- // Appium
- try {
- Object osProcess = ReflectionHelpers.getPrivateFieldValue(
- process.getClass(), process, "process", Object.class
- );
- Object watchdog = ReflectionHelpers.getPrivateFieldValue(
- osProcess.getClass(), osProcess, "executeWatchdog", Object.class
- );
- Process nativeProcess = ReflectionHelpers.getPrivateFieldValue(
- watchdog.getClass(), watchdog, "process", Process.class
- );
- nativeProcess.destroy();
- nativeProcess.waitFor(timeout.toMillis(), TimeUnit.MILLISECONDS);
- } catch (Exception e) {
- LOG.warn("No explicit timeout could be applied to the process termination", e);
- }
-
- return process.destroy();
- }
-
  /**
  * Destroys the service.
- * This methods waits up to `DESTROY_TIMEOUT` seconds for the Appium service
+ * This method waits up to `DESTROY_TIMEOUT` seconds for the Appium service
  * to exit gracefully.
  */
  private void destroyProcess() {
- destroyProcess(DESTROY_TIMEOUT);
+ if (process == null || !process.isAlive()) {
+ return;
+ }
+ process.shutdown(DESTROY_TIMEOUT);
  }
 
  /**
@@ -277,11 +244,7 @@ private void destroyProcess() {
  */
  @Nullable
  public String getStdOut() {
- if (process != null) {
- return process.getStdOut();
- }
-
- return null;
+ return ofNullable(process).map(ExternalProcess::getOutput).orElse(null);
  }
 
  /**