diff --git a/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java b/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java index 91f180401..374c73464 100644 --- a/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java +++ b/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java @@ -44,7 +44,7 @@ public final class AppiumDriverLocalService extends DriverService { private final String ipAddress; private final long startupTimeout; private final TimeUnit timeUnit; - private final ReentrantLock lock = new ReentrantLock(); + private final ReentrantLock lock = new ReentrantLock(true); //uses "fair" thread ordering policy private final ListOutputStream stream = new ListOutputStream().add(System.out); @@ -82,19 +82,21 @@ public URL getUrl() { @Override public boolean isRunning() { lock.lock(); - if (process == null) { - return false; - } + try { + if (process == null) { + return false; + } - if (!process.isRunning()) { - return false; - } + if (!process.isRunning()) { + return false; + } - try { - ping(500, TimeUnit.MILLISECONDS); - return true; - } catch (UrlChecker.TimeoutException e) { - return false; + try { + ping(500, TimeUnit.MILLISECONDS); + return true; + } catch (UrlChecker.TimeoutException e) { + return false; + } } finally { lock.unlock(); } @@ -118,27 +120,30 @@ private void ping(long time, TimeUnit timeUnit) throws UrlChecker.TimeoutExcepti */ public void start() throws AppiumServerHasNotBeenStartedLocallyException { lock.lock(); - if (isRunning()) - return; - try { - process = new CommandLine(this.nodeJSExec.getCanonicalPath(), nodeJSArgs.toArray(new String[] {})); - process.setEnvironmentVariables(nodeJSEnvironment); - process.copyOutputTo(stream); - process.executeAsync(); - ping(startupTimeout, timeUnit); - } catch (Throwable e) { - destroyProcess(); - String msgTxt = "The local appium server has not been started. " + - "The given Node.js executable: " + this.nodeJSExec.getAbsolutePath() + " Arguments: " + nodeJSArgs.toString() + " " + "\n"; - if (process != null) { - String processStream = process.getStdOut(); - if (!StringUtils.isBlank(processStream)) - msgTxt = msgTxt + "Process output: " + processStream + "\n"; + if (isRunning()) { + return; } - throw new AppiumServerHasNotBeenStartedLocallyException(msgTxt, - e); + try { + process = new CommandLine(this.nodeJSExec.getCanonicalPath(), nodeJSArgs.toArray(new String[]{})); + process.setEnvironmentVariables(nodeJSEnvironment); + process.copyOutputTo(stream); + process.executeAsync(); + ping(startupTimeout, timeUnit); + } catch (Throwable e) { + destroyProcess(); + String msgTxt = "The local appium server has not been started. " + + "The given Node.js executable: " + this.nodeJSExec.getAbsolutePath() + " Arguments: " + nodeJSArgs.toString() + " " + "\n"; + if (process != null) { + String processStream = process.getStdOut(); + if (!StringUtils.isBlank(processStream)) + msgTxt = msgTxt + "Process output: " + processStream + "\n"; + } + + throw new AppiumServerHasNotBeenStartedLocallyException(msgTxt, + e); + } } finally { lock.unlock(); } @@ -153,14 +158,22 @@ public void start() throws AppiumServerHasNotBeenStartedLocallyException { @Override public void stop() { lock.lock(); - destroyProcess(); - lock.unlock(); + try { + if (process != null) { + destroyProcess(); + } + process = null; + } + finally { + lock.unlock(); + } } private void destroyProcess(){ - if (process != null) + if (process.isRunning()) { process.destroy(); + } } /** diff --git a/src/test/java/io/appium/java_client/localserver/ServerBuilderTest.java b/src/test/java/io/appium/java_client/localserver/ServerBuilderTest.java index 99b001fa0..52b689435 100644 --- a/src/test/java/io/appium/java_client/localserver/ServerBuilderTest.java +++ b/src/test/java/io/appium/java_client/localserver/ServerBuilderTest.java @@ -179,4 +179,35 @@ public void checkAbilityToChangeOutputStreamAfterTheServiceIsStarted() throws Ex file.delete(); } } + + @Test + public void checkAbilityToShutDownService() { + AppiumDriverLocalService service = AppiumDriverLocalService.buildDefaultService(); + service.start(); + service.stop(); + assertTrue(!service.isRunning()); + } + + @Test + public void checkAbilityToStartAndShutDownFewServices() throws Exception{ + AppiumDriverLocalService service1 = new AppiumServiceBuilder().usingAnyFreePort().build(); + AppiumDriverLocalService service2 = new AppiumServiceBuilder().usingAnyFreePort().build(); + AppiumDriverLocalService service3 = new AppiumServiceBuilder().usingAnyFreePort().build(); + AppiumDriverLocalService service4 = new AppiumServiceBuilder().usingAnyFreePort().build(); + service1.start(); + service2.start(); + service3.start(); + service4.start(); + service1.stop(); + Thread.sleep(1000); + service2.stop(); + Thread.sleep(1000); + service3.stop(); + Thread.sleep(1000); + service4.stop(); + assertTrue(!service1.isRunning()); + assertTrue(!service2.isRunning()); + assertTrue(!service3.isRunning()); + assertTrue(!service4.isRunning()); + } } diff --git a/src/test/java/io/appium/java_client/localserver/ThreadSafetyTest.java b/src/test/java/io/appium/java_client/localserver/ThreadSafetyTest.java new file mode 100644 index 000000000..60b3c3892 --- /dev/null +++ b/src/test/java/io/appium/java_client/localserver/ThreadSafetyTest.java @@ -0,0 +1,240 @@ +package io.appium.java_client.localserver; + + +import io.appium.java_client.service.local.AppiumDriverLocalService; +import org.junit.Test; + +import static org.junit.Assert.assertTrue; + +public class ThreadSafetyTest { + + private static abstract class Action implements Cloneable { + abstract Object perform(); + + public Action clone() { + try { + return (Action) super.clone(); + } + catch (Throwable t) { + throw new RuntimeException(t); + } + } + } + + private static class TestThread implements Runnable { + private final Action action; + private Object result; + private Throwable t; + + TestThread(Action action) { + this.action = action; + } + + @Override + public void run() { + try { + result = action.perform(); + } + catch (Throwable t) { + this.t = t; + } + + } + } + + final AppiumDriverLocalService service = AppiumDriverLocalService.buildDefaultService(); + final Action run = new Action() { + @Override + Object perform() { + service.start(); + return "OK"; + } + }; + final Action run2 = run.clone(); + + final Action isRunning = new Action() { + @Override + Object perform() { + return service.isRunning(); + } + }; + final Action isRunning2 = isRunning.clone(); + + final Action stop = new Action() { + @Override + Object perform() { + service.stop(); + return "OK"; + } + }; + + final Action stop2 = stop.clone(); + + @Test + public void whenFewTreadsDoTheSameWork() throws Throwable { + + TestThread runTestThread = new TestThread(run); + TestThread runTestThread2 = new TestThread(run2); + + TestThread isRunningTestThread = new TestThread(isRunning); + TestThread isRunningTestThread2 = new TestThread(isRunning2); + + TestThread stopTestThread = new TestThread(stop); + TestThread stopTestThread2 = new TestThread(stop2); + + Thread runThread = new Thread(runTestThread); + Thread runThread2 = new Thread(runTestThread2); + + Thread isRunningThread = new Thread(isRunningTestThread); + Thread isRunningThread2 = new Thread(isRunningTestThread2); + + Thread stopThread = new Thread(stopTestThread); + Thread stopThread2 = new Thread(stopTestThread2); + + try { + runThread.start(); + runThread2.start(); + + while (runThread.isAlive() || runThread2.isAlive()) { + //do nothing + } + + if (runTestThread.t != null) { + throw runTestThread.t; + } + + if (runTestThread2.t != null) { + throw runTestThread2.t; + } + + assertTrue(runTestThread.result.equals("OK")); + assertTrue(runTestThread2.result.equals("OK")); + assertTrue(service.isRunning()); + + isRunningThread.start(); + isRunningThread2.start(); + + while (isRunningThread.isAlive() || isRunningThread2.isAlive()) { + //do nothing + } + + if (isRunningTestThread.t != null) { + throw isRunningTestThread.t; + } + + if (isRunningTestThread2.t != null) { + throw isRunningTestThread2.t; + } + + assertTrue(isRunningTestThread.result.equals(true)); + assertTrue(isRunningTestThread2.result.equals(true)); + + stopThread.start(); + stopThread2.start(); + + while (stopThread.isAlive() || stopThread2.isAlive()) { + //do nothing + } + + if (stopTestThread.t != null) { + throw stopTestThread.t; + } + + if (stopTestThread2.t != null) { + throw stopTestThread2.t; + } + + assertTrue(stopTestThread.result.equals("OK")); + assertTrue(stopTestThread2.result.equals("OK")); + assertTrue(!service.isRunning()); + } + finally { + if (service.isRunning()) { + service.stop(); + } + } + + } + + @Test + public void whenFewTreadsDoDifferentWork() throws Throwable { + TestThread runTestThread = new TestThread(run); + TestThread runTestThread2 = new TestThread(run2); + + TestThread isRunningTestThread = new TestThread(isRunning); + TestThread isRunningTestThread2 = new TestThread(isRunning2); + + TestThread stopTestThread = new TestThread(stop); + TestThread stopTestThread2 = new TestThread(stop2); + + Thread runThread = new Thread(runTestThread); + Thread runThread2 = new Thread(runTestThread2); + + Thread isRunningThread = new Thread(isRunningTestThread); + Thread isRunningThread2 = new Thread(isRunningTestThread2); + + Thread stopThread = new Thread(stopTestThread); + Thread stopThread2 = new Thread(stopTestThread2); + + try { + runThread.start(); //(1) + Thread.sleep(10); + isRunningThread.start();//(2) + Thread.sleep(10); + stopThread.start(); //(3) + + while (runThread.isAlive() || isRunningThread.isAlive() || stopThread.isAlive()) { + //do nothing + } + + if (runTestThread.t != null) { + throw runTestThread.t; + } + + if (isRunningTestThread.t != null) { + throw isRunningTestThread.t; + } + + if (stopTestThread.t != null) { + throw stopTestThread.t; + } + + assertTrue(runTestThread.result.equals("OK")); //the service had been started firstly (see (1)) + assertTrue(isRunningTestThread.result.equals(true)); //it was running (see (2)) + assertTrue(stopTestThread.result.equals("OK")); //and then the test tried to shut down it (see (3)) + assertTrue(!service.isRunning()); + + isRunningThread2.start(); // (1) + Thread.sleep(10); + stopThread2.start(); // (2) + Thread.sleep(10); + runThread2.start(); //(3) + + while (runThread2.isAlive() || isRunningThread2.isAlive() || stopThread2.isAlive()) { + //do nothing + } + + if (runTestThread2.t != null) { + throw runTestThread.t; + } + + if (isRunningTestThread2.t != null) { + throw isRunningTestThread.t; + } + + if (stopTestThread2.t != null) { + throw stopTestThread.t; + } + + assertTrue(isRunningTestThread2.result.equals(false)); //the service wasn't being running (see (1)) + assertTrue(stopTestThread2.result.equals("OK")); //the service had not been started firstly (see (2)), it is ok + assertTrue(runTestThread2.result.equals("OK")); //and then it was started (see (3)) + assertTrue(service.isRunning()); + } + finally { + if (service.isRunning()) { + service.stop(); + } + } + } +}