Clean up immediately if starting a driver or session errors in RemoteWebDriver.

Fixes SeleniumHQ#6811: Grid nodes create more browsers than limit in configuration.

Author: nirvdrum

java/client/src/org/openqa/selenium/remote/RemoteWebDriver.java

@@ -109,8 +109,22 @@ public RemoteWebDriver(CommandExecutor executor, Capabilities desiredCapabilitie
  if (executor instanceof NeedsLocalLogs) {
  ((NeedsLocalLogs)executor).setLocalLogs(localLogs);
  }
- startClient();
- startSession(desiredCapabilities, requiredCapabilities);
+
+ try {
+ startClient();
+ } catch (Exception e) {
+ stopClient();
+
+ throw new WebDriverException("Failed to start client.", e);
+ }
+
+ try {
+ startSession(desiredCapabilities, requiredCapabilities);
+ } catch (Exception e) {
+ quit();
+
+ throw new WebDriverException("Failed to start session.", e);
+ }
  }
 
  public RemoteWebDriver(CommandExecutor executor, Capabilities desiredCapabilities) {

java/server/test/org/openqa/selenium/remote/server/RemoteWebDriverTest.java

@@ -24,6 +24,8 @@
 import org.json.JSONException;
 import org.json.JSONObject;
 import org.junit.Test;
+import org.openqa.selenium.Capabilities;
+import org.openqa.selenium.WebDriverException;
 import org.openqa.selenium.remote.CapabilityType;
 import org.openqa.selenium.remote.CommandExecutor;
 import org.openqa.selenium.remote.ErrorCodes;
@@ -34,8 +36,11 @@
 import java.io.IOException;
 import java.net.HttpURLConnection;
 import java.net.URL;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 public class RemoteWebDriverTest extends JUnit4TestBase {
+ private boolean stopClientCalled = false;
+ private boolean quitCalled = false;
 
  @Test
  public void testCanCheckServerStatusIndependentlyOfSessions() throws IOException, JSONException {
@@ -78,10 +83,95 @@ public void testCanCheckServerStatusIndependentlyOfSessions() throws IOException
  }
  }
 
+ @Test
+ public void testStopsClientIfStartClientFails() {
+ if (!(driver instanceof RemoteWebDriver)) {
+ System.out.println("Skipping test: driver is not a remote webdriver");
+ return;
+ }
+
+ RemoteWebDriver remote = (RemoteWebDriver) driver;
+ boolean exceptionThrown = false;
+ AtomicBoolean stopCalled = new AtomicBoolean(false);
+
+ try {
+ new BadStartClientRemoteWebDriver(remote.getCommandExecutor(),
+ remote.getCapabilities(),
+ remote.getCapabilities(),
+ stopCalled);
+ } catch (WebDriverException e) {
+ assertTrue(e.getMessage().contains("Failed to start client."));
+
+ exceptionThrown = true;
+ }
+
+ assertTrue(exceptionThrown);
+ assertTrue(stopClientCalled);
+ }
+
+ @Test
+ public void testQuitsIfStartSessionFails() {
+ if (!(driver instanceof RemoteWebDriver)) {
+ System.out.println("Skipping test: driver is not a remote webdriver");
+ return;
+ }
+
+ RemoteWebDriver remote = (RemoteWebDriver) driver;
+ boolean exceptionThrown = false;
+
+ try {
+ new BadStartSessionRemoteWebDriver(remote.getCommandExecutor(),
+ remote.getCapabilities(),
+ remote.getCapabilities());
+ } catch (WebDriverException e) {
+ assertTrue(e.getMessage().contains("Failed to start session."));
+
+ exceptionThrown = true;
+ }
+
+ assertTrue(exceptionThrown);
+ assertTrue(quitCalled);
+ }
+
  private static void assertHasKeys(JSONObject object, String... keys) {
  for (String key : keys) {
  assertTrue("Object does not contain expected key: " + key + " (" + object + ")",
  object.has(key));
  }
  }
+
+ private class BadStartClientRemoteWebDriver extends RemoteWebDriver {
+ public BadStartClientRemoteWebDriver(CommandExecutor executor, Capabilities desiredCapabilities,
+ Capabilities requiredCapabilities, AtomicBoolean stopCalled) {
+ super(executor, desiredCapabilities, requiredCapabilities);
+ }
+
+ @Override
+ protected void startClient() {
+ throw new RuntimeException("Stub client that should fail");
+ }
+
+ @Override
+ protected void stopClient() {
+ stopClientCalled = true;
+ }
+ }
+
+ private class BadStartSessionRemoteWebDriver extends RemoteWebDriver {
+ public BadStartSessionRemoteWebDriver(CommandExecutor executor, Capabilities desiredCapabilities,
+ Capabilities requiredCapabilities) {
+ super(executor, desiredCapabilities, requiredCapabilities);
+ }
+
+ @Override
+ protected void startSession(Capabilities desiredCapabilities,
+ Capabilities requiredCapabilities) {
+ throw new RuntimeException("Stub session that should fail");
+ }
+
+ @Override
+ public void quit() {
+ quitCalled = true;
+ }
+ }
 }