From bdb8242f57a2bb742c49d49e9a73c4400be59ed8 Mon Sep 17 00:00:00 2001 From: hollingsworthd Date: Wed, 27 Apr 2016 23:38:26 -0400 Subject: [PATCH] Use 3 ports per process to avoid conflicts. Handle failures more gracefully on startup of RMI. --- .../jbrowserdriver/JBrowserDriver.java | 129 ++++++++++-------- .../jbrowserdriver/JBrowserDriverServer.java | 116 ++++++++-------- .../jbrowserdriver/PortGroup.java | 49 +++++++ .../jbrowserdriver/Settings.java | 128 +++++++++-------- .../jbrowserdriver/SocketFactory.java | 75 ++++++---- .../jbrowserdriver/diagnostics/Test.java | 2 +- 6 files changed, 297 insertions(+), 202 deletions(-) create mode 100644 src/com/machinepublishers/jbrowserdriver/PortGroup.java diff --git a/src/com/machinepublishers/jbrowserdriver/JBrowserDriver.java b/src/com/machinepublishers/jbrowserdriver/JBrowserDriver.java index 2e904666..e4b36145 100644 --- a/src/com/machinepublishers/jbrowserdriver/JBrowserDriver.java +++ b/src/com/machinepublishers/jbrowserdriver/JBrowserDriver.java @@ -35,7 +35,6 @@ import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; import java.util.jar.Attributes; @@ -101,8 +100,8 @@ public class JBrowserDriver extends RemoteWebDriver implements Killable { } intercept = interceptTmp; } - private static final Set childPortsAvailable = new LinkedHashSet(); - private static final Set childPortsUsed = new LinkedHashSet(); + private static final Set portGroupsAvailable = new LinkedHashSet(); + private static final Set portGroupsUsed = new LinkedHashSet(); private static final List args; private static final List waiting = new ArrayList(); private static int curWaiting; @@ -115,9 +114,9 @@ public class JBrowserDriver extends RemoteWebDriver implements Killable { private final JBrowserDriverRemote remote; private final Logs logs; private final AtomicReference process = new AtomicReference(); - private final AtomicInteger configuredChildPort = new AtomicInteger(); - private final AtomicInteger actualChildPort = new AtomicInteger(); - private final AtomicInteger actualParentPort = new AtomicInteger(); + private final AtomicBoolean processEnded = new AtomicBoolean(); + private final AtomicReference configuredPortGroup = new AtomicReference(); + private final AtomicReference actualPortGroup = new AtomicReference(); private final AtomicReference options = new AtomicReference(); private final SessionId sessionId; private final SocketLock lock = new SocketLock(); @@ -235,20 +234,20 @@ public JBrowserDriver(final Settings settings) { this.shutdownHook = new FileRemover(tmpDir); Runtime.getRuntime().addShutdownHook(shutdownHook); - synchronized (childPortsAvailable) { - for (int curPort : settings.childPorts()) { - if (!childPortsAvailable.contains(curPort) && !childPortsUsed.contains(curPort)) { - childPortsAvailable.add(curPort); + synchronized (portGroupsAvailable) { + for (PortGroup curPortGroup : settings.portGroups()) { + if (!portGroupsAvailable.contains(curPortGroup) && !portGroupsUsed.contains(curPortGroup)) { + portGroupsAvailable.add(curPortGroup); } } waiting.add(key); while (true) { boolean ready = false; curWaiting = curWaiting >= waiting.size() ? 0 : curWaiting; - if (key.equals(waiting.get(curWaiting)) && !childPortsAvailable.isEmpty()) { - for (int curPort : settings.childPorts()) { - if (childPortsAvailable.contains(curPort)) { - configuredChildPort.set(curPort); + if (key.equals(waiting.get(curWaiting)) && !portGroupsAvailable.isEmpty()) { + for (PortGroup curPortGroup : settings.portGroups()) { + if (portGroupsAvailable.contains(curPortGroup)) { + configuredPortGroup.set(curPortGroup); ready = true; break; } @@ -258,25 +257,30 @@ public JBrowserDriver(final Settings settings) { break; } else { ++curWaiting; - childPortsAvailable.notifyAll(); + portGroupsAvailable.notifyAll(); } } try { - childPortsAvailable.wait(); + portGroupsAvailable.wait(); } catch (InterruptedException e) {} } waiting.remove(key); - childPortsAvailable.remove(configuredChildPort.get()); - childPortsUsed.add(configuredChildPort.get()); + portGroupsAvailable.remove(configuredPortGroup.get()); + portGroupsUsed.add(configuredPortGroup.get()); + } + sessionId = new SessionId(launchProcess(settings.host(), configuredPortGroup.get(), settings.logger())); + if (actualPortGroup.get() == null) { + Util.handleException(new IllegalStateException("Could not launch browser.")); } - launchProcess(settings.host(), settings.parentPort(), configuredChildPort.get(), settings.logger()); JBrowserDriverRemote instanceTmp = null; try { - instanceTmp = (JBrowserDriverRemote) LocateRegistry - .getRegistry(settings.host(), actualChildPort.get(), - new SocketFactory(settings.host(), actualParentPort.get(), actualChildPort.get(), lock)) - .lookup("JBrowserDriverRemote"); - instanceTmp.setUp(settings); + synchronized (lock) { + instanceTmp = (JBrowserDriverRemote) LocateRegistry + .getRegistry(settings.host(), (int) actualPortGroup.get().child, + new SocketFactory(settings.host(), actualPortGroup.get(), lock)) + .lookup("JBrowserDriverRemote"); + instanceTmp.setUp(settings); + } } catch (Throwable t) { Util.handleException(t); } @@ -289,15 +293,6 @@ public JBrowserDriver(final Settings settings) { } catch (Throwable t) { Util.handleException(t); } - sessionId = new SessionId(new StringBuilder() - .append("[Instance ") - .append(sessionIdCounter.incrementAndGet()) - .append("][Port ") - .append(actualChildPort.get()) - .append("]") - .append(actualChildPort.get() == configuredChildPort.get() - ? "" : "[Process " + Math.abs(configuredChildPort.get()) + "]") - .toString()); logs = new Logs(logsRemote, lock); } @@ -312,7 +307,7 @@ protected void finalize() throws Throwable { } catch (Throwable t) {} } - private void launchProcess(final String host, final int parentPort, final int childPort, final Logger logger) { + private String launchProcess(final String host, final PortGroup portGroup, final Logger logger) { final AtomicBoolean ready = new AtomicBoolean(); final AtomicReference logPrefix = new AtomicReference(""); new Thread(new Runnable() { @@ -322,8 +317,9 @@ public void run() { myArgs.add("-Djava.io.tmpdir=" + tmpDir.getAbsolutePath()); myArgs.add("-Djava.rmi.server.hostname=" + host); myArgs.add(JBrowserDriverServer.class.getName()); - myArgs.add(Integer.toString(parentPort)); - myArgs.add(Integer.toString(childPort)); + myArgs.add(Long.toString(portGroup.child)); + myArgs.add(Long.toString(portGroup.parent)); + myArgs.add(Long.toString(portGroup.parentAlt)); try { new ProcessExecutor() .environment(System.getenv()) @@ -351,17 +347,19 @@ protected void processLine(String line) { if (line != null && !line.isEmpty()) { if (!done) { synchronized (ready) { - if (line.startsWith("parent on port ")) { - actualParentPort.set(Integer.parseInt( - line.substring("parent on port ".length()))); - } else if (line.startsWith("child on port ")) { - actualChildPort.set(Integer.parseInt( - line.substring("child on port ".length()))); - String portId = actualChildPort.get() != childPort - ? actualChildPort.get() + "; Process " + Math.abs(childPort) : Integer.toString(childPort); - logPrefix.set("[Port " + portId + "] "); + if (line.startsWith("ready on ports ")) { + String[] parts = line.substring("ready on ports ".length()).split("/"); + actualPortGroup.set(new PortGroup( + Integer.parseInt(parts[0]), Integer.parseInt(parts[1]), Integer.parseInt(parts[2]))); + logPrefix.set(new StringBuilder() + .append("[Instance ") + .append(sessionIdCounter.incrementAndGet()) + .append("][Port ") + .append(actualPortGroup.get().child) + .append("]") + .toString()); ready.set(true); - ready.notify(); + ready.notifyAll(); done = true; } else { log(logger, logPrefix.get(), line); @@ -384,7 +382,12 @@ protected void processLine(String line) { } catch (Throwable t) { Util.handleException(t); } + endProcess(); FileUtils.deleteQuietly(tmpDir); + synchronized (ready) { + ready.set(true); + ready.notifyAll(); + } } }).start(); synchronized (ready) { @@ -395,6 +398,7 @@ protected void processLine(String line) { } catch (InterruptedException e) {} } } + return logPrefix.get(); } private static void log(Logger logger, String prefix, String message) { @@ -402,11 +406,13 @@ private static void log(Logger logger, String prefix, String message) { LogRecord record = null; if (message.startsWith(">")) { String[] parts = message.substring(1).split("/", 3); - record = new LogRecord(Level.parse(parts[0]), prefix + parts[2]); + record = new LogRecord(Level.parse(parts[0]), + new StringBuilder().append(prefix).append(" ").append(parts[2]).toString()); record.setSourceMethodName(parts[1]); record.setSourceClassName(JBrowserDriver.class.getName()); } else { - record = new LogRecord(Level.WARNING, prefix + message); + record = new LogRecord(Level.WARNING, + new StringBuilder().append(prefix).append(" ").append(message).toString()); record.setSourceMethodName(null); record.setSourceClassName(JBrowserDriver.class.getName()); } @@ -1061,18 +1067,23 @@ public Navigation navigate() { } private void endProcess() { - try { - PidProcess pidProcess = Processes.newPidProcess(process.get()); - if (!pidProcess.destroyGracefully().waitFor(10, TimeUnit.SECONDS)) { - pidProcess.destroyForcefully(); + if (processEnded.compareAndSet(false, true)) { + final Process proc = process.get(); + if (proc != null) { + try { + PidProcess pidProcess = Processes.newPidProcess(proc); + if (!pidProcess.destroyGracefully().waitFor(10, TimeUnit.SECONDS)) { + pidProcess.destroyForcefully(); + } + } catch (Throwable t2) { + proc.destroyForcibly(); + } + } + synchronized (portGroupsAvailable) { + portGroupsAvailable.add(configuredPortGroup.get()); + portGroupsUsed.remove(configuredPortGroup.get()); + portGroupsAvailable.notifyAll(); } - } catch (Throwable t2) { - process.get().destroyForcibly(); - } - synchronized (childPortsAvailable) { - childPortsAvailable.add(configuredChildPort.get()); - childPortsUsed.remove(configuredChildPort.get()); - childPortsAvailable.notifyAll(); } } diff --git a/src/com/machinepublishers/jbrowserdriver/JBrowserDriverServer.java b/src/com/machinepublishers/jbrowserdriver/JBrowserDriverServer.java index b17ee360..14cf4267 100644 --- a/src/com/machinepublishers/jbrowserdriver/JBrowserDriverServer.java +++ b/src/com/machinepublishers/jbrowserdriver/JBrowserDriverServer.java @@ -64,7 +64,6 @@ class JBrowserDriverServer extends RemoteObject implements JBrowserDriverRemote, WebDriver, JavascriptExecutor, FindsById, FindsByClassName, FindsByLinkText, FindsByName, FindsByCssSelector, FindsByTagName, FindsByXPath, HasInputDevices, HasCapabilities, TakesScreenshot, Killable { - private static final AtomicInteger parentPort = new AtomicInteger(); private static final AtomicInteger childPort = new AtomicInteger(); private static final AtomicReference socketFactory = new AtomicReference(); private static Registry registry; @@ -73,7 +72,6 @@ class JBrowserDriverServer extends RemoteObject implements JBrowserDriverRemote, * RMI entry point. */ public static void main(String[] args) { - System.setProperty("sun.rmi.dgc.server.gcInterval", Long.toString(Long.MAX_VALUE)); CookieManager.setDefault(new CookieStore()); try { URL.setURLStreamHandlerFactory(new StreamHandler()); @@ -98,8 +96,9 @@ public static void main(String[] args) { } } final String host = System.getProperty("java.rmi.server.hostname"); - parentPort.set(Integer.parseInt(args[0])); - childPort.set(Integer.parseInt(args[1])); + childPort.set((int) Long.parseLong(args[0])); + long parentPort = Long.parseLong(args[1]); + long parentAltPort = Long.parseLong(args[2]); Registry registryTmp = null; final int maxTries = 5; for (int i = 1; i <= maxTries; i++) { @@ -107,10 +106,14 @@ public static void main(String[] args) { if (childPort.get() <= 0) { childPort.set(findPort(host)); } - if (parentPort.get() <= 0) { - parentPort.set(findPort(host)); + if (parentPort <= 0) { + parentPort = findPort(host); } - socketFactory.set(new SocketFactory(host, parentPort.get(), childPort.get(), new SocketLock())); + if (parentAltPort <= 0) { + parentAltPort = findPort(host); + } + socketFactory.set(new SocketFactory(host, + new PortGroup(childPort.get(), parentPort, parentAltPort), new SocketLock())); registryTmp = LocateRegistry.createRegistry(childPort.get(), socketFactory.get(), socketFactory.get()); registryTmp.rebind("JBrowserDriverRemote", new JBrowserDriverServer()); break; @@ -127,8 +130,7 @@ public static void main(String[] args) { } catch (IOException e) { Util.handleException(e); } - System.out.println("parent on port " + parentPort.get()); - System.out.println("child on port " + childPort.get()); + System.out.println("ready on ports " + childPort.get() + "/" + parentPort + "/" + parentAltPort); } private static int findPort(String host) throws IOException { @@ -156,13 +158,13 @@ static SocketFactory socketFactory() { public JBrowserDriverServer() throws RemoteException {} @Override - public synchronized void setUp(final Settings settings) { + public void setUp(final Settings settings) { SettingsManager.register(settings); context.set(new Context()); } @Override - public synchronized void storeCapabilities(final Capabilities capabilities) { + public void storeCapabilities(final Capabilities capabilities) { context.get().capabilities.set(capabilities); } @@ -170,7 +172,7 @@ public synchronized void storeCapabilities(final Capabilities capabilities) { * Optionally call this method if you want JavaFX initialized and the browser * window opened immediately. Otherwise, initialization will happen lazily. */ - public synchronized void init() { + public void init() { context.get().init(this); } @@ -181,7 +183,7 @@ public synchronized void init() { * @param settings * New settings to take effect, superseding the original ones */ - public synchronized void reset(final Settings settings) { + public void reset(final Settings settings) { AppThread.exec(Pause.SHORT, new AtomicInteger(-1), new Sync() { @Override public Object perform() { @@ -201,7 +203,7 @@ public Object perform() { * Reset the state of the browser. More efficient than quitting the * browser and creating a new instance. */ - public synchronized void reset() { + public void reset() { reset(SettingsManager.settings()); } @@ -209,7 +211,7 @@ public synchronized void reset() { * {@inheritDoc} */ @Override - public synchronized String getPageSource() { + public String getPageSource() { init(); WebElement element = ElementServer.create(context.get()).findElementByTagName("html"); if (element != null) { @@ -230,7 +232,7 @@ public synchronized String getPageSource() { * {@inheritDoc} */ @Override - public synchronized String getCurrentUrl() { + public String getCurrentUrl() { init(); return AppThread.exec(Pause.NONE, context.get().statusCode, new Sync() { public String perform() { @@ -243,16 +245,16 @@ public String perform() { * {@inheritDoc} */ @Override - public synchronized void pageWait() { + public void pageWait() { context.get().item().httpListener.get().resetStatusCode(); getStatusCode(); } - public synchronized int getStatusCode() { + public int getStatusCode() { return getStatusCode(context.get().timeouts.get().getPageLoadTimeoutMS()); } - private synchronized int getStatusCode(long waitMS) { + private int getStatusCode(long waitMS) { init(); try { synchronized (context.get().statusCode) { @@ -270,7 +272,7 @@ private synchronized int getStatusCode(long waitMS) { * {@inheritDoc} */ @Override - public synchronized String getTitle() { + public String getTitle() { init(); return AppThread.exec(Pause.NONE, context.get().statusCode, new Sync() { public String perform() { @@ -283,7 +285,7 @@ public String perform() { * {@inheritDoc} */ @Override - public synchronized void get(final String url) { + public void get(final String url) { init(); long start = System.currentTimeMillis(); try { @@ -323,7 +325,7 @@ public Object perform() { * {@inheritDoc} */ @Override - public synchronized ElementServer findElement(By by) { + public ElementServer findElement(By by) { init(); return ElementServer.create(context.get()).findElement(by); } @@ -332,7 +334,7 @@ public synchronized ElementServer findElement(By by) { * {@inheritDoc} */ @Override - public synchronized List findElements(By by) { + public List findElements(By by) { init(); return ElementServer.create(context.get()).findElements(by); } @@ -341,7 +343,7 @@ public synchronized List findElements(By by) { * {@inheritDoc} */ @Override - public synchronized ElementServer findElementById(String id) { + public ElementServer findElementById(String id) { init(); return ElementServer.create(context.get()).findElementById(id); } @@ -350,7 +352,7 @@ public synchronized ElementServer findElementById(String id) { * {@inheritDoc} */ @Override - public synchronized List findElementsById(String id) { + public List findElementsById(String id) { init(); return ElementServer.create(context.get()).findElementsById(id); } @@ -359,7 +361,7 @@ public synchronized List findElementsById(String id) { * {@inheritDoc} */ @Override - public synchronized ElementServer findElementByXPath(String expr) { + public ElementServer findElementByXPath(String expr) { init(); return ElementServer.create(context.get()).findElementByXPath(expr); } @@ -368,7 +370,7 @@ public synchronized ElementServer findElementByXPath(String expr) { * {@inheritDoc} */ @Override - public synchronized List findElementsByXPath(String expr) { + public List findElementsByXPath(String expr) { init(); return ElementServer.create(context.get()).findElementsByXPath(expr); } @@ -386,7 +388,7 @@ public ElementServer findElementByLinkText(final String text) { * {@inheritDoc} */ @Override - public synchronized ElementServer findElementByPartialLinkText(String text) { + public ElementServer findElementByPartialLinkText(String text) { init(); return ElementServer.create(context.get()).findElementByPartialLinkText(text); } @@ -395,7 +397,7 @@ public synchronized ElementServer findElementByPartialLinkText(String text) { * {@inheritDoc} */ @Override - public synchronized List findElementsByLinkText(String text) { + public List findElementsByLinkText(String text) { init(); return ElementServer.create(context.get()).findElementsByLinkText(text); } @@ -404,7 +406,7 @@ public synchronized List findElementsByLinkText(String text) { * {@inheritDoc} */ @Override - public synchronized List findElementsByPartialLinkText(String text) { + public List findElementsByPartialLinkText(String text) { init(); return ElementServer.create(context.get()).findElementsByPartialLinkText(text); } @@ -413,7 +415,7 @@ public synchronized List findElementsByPartialLinkText(String text) { * {@inheritDoc} */ @Override - public synchronized ElementServer findElementByClassName(String cssClass) { + public ElementServer findElementByClassName(String cssClass) { init(); return ElementServer.create(context.get()).findElementByClassName(cssClass); } @@ -422,7 +424,7 @@ public synchronized ElementServer findElementByClassName(String cssClass) { * {@inheritDoc} */ @Override - public synchronized List findElementsByClassName(String cssClass) { + public List findElementsByClassName(String cssClass) { init(); return ElementServer.create(context.get()).findElementsByClassName(cssClass); } @@ -431,7 +433,7 @@ public synchronized List findElementsByClassName(String cssClass) { * {@inheritDoc} */ @Override - public synchronized ElementServer findElementByName(String name) { + public ElementServer findElementByName(String name) { init(); return ElementServer.create(context.get()).findElementByName(name); } @@ -440,7 +442,7 @@ public synchronized ElementServer findElementByName(String name) { * {@inheritDoc} */ @Override - public synchronized List findElementsByName(String name) { + public List findElementsByName(String name) { init(); return ElementServer.create(context.get()).findElementsByName(name); } @@ -449,7 +451,7 @@ public synchronized List findElementsByName(String name) { * {@inheritDoc} */ @Override - public synchronized ElementServer findElementByCssSelector(String expr) { + public ElementServer findElementByCssSelector(String expr) { init(); return ElementServer.create(context.get()).findElementByCssSelector(expr); } @@ -458,7 +460,7 @@ public synchronized ElementServer findElementByCssSelector(String expr) { * {@inheritDoc} */ @Override - public synchronized List findElementsByCssSelector(String expr) { + public List findElementsByCssSelector(String expr) { init(); return ElementServer.create(context.get()).findElementsByCssSelector(expr); } @@ -467,7 +469,7 @@ public synchronized List findElementsByCssSelector(String expr) { * {@inheritDoc} */ @Override - public synchronized ElementServer findElementByTagName(String tagName) { + public ElementServer findElementByTagName(String tagName) { init(); return ElementServer.create(context.get()).findElementByTagName(tagName); } @@ -476,7 +478,7 @@ public synchronized ElementServer findElementByTagName(String tagName) { * {@inheritDoc} */ @Override - public synchronized List findElementsByTagName(String tagName) { + public List findElementsByTagName(String tagName) { init(); return ElementServer.create(context.get()).findElementsByTagName(tagName); } @@ -485,7 +487,7 @@ public synchronized List findElementsByTagName(String tagName) { * {@inheritDoc} */ @Override - public synchronized Object executeAsyncScript(String script, Object... args) { + public Object executeAsyncScript(String script, Object... args) { init(); return ElementServer.create(context.get()).executeAsyncScript(script, args); } @@ -494,7 +496,7 @@ public synchronized Object executeAsyncScript(String script, Object... args) { * {@inheritDoc} */ @Override - public synchronized Object executeScript(String script, Object... args) { + public Object executeScript(String script, Object... args) { init(); return ElementServer.create(context.get()).executeScript(script, args); } @@ -503,7 +505,7 @@ public synchronized Object executeScript(String script, Object... args) { * {@inheritDoc} */ @Override - public synchronized KeyboardServer getKeyboard() { + public KeyboardServer getKeyboard() { init(); return context.get().keyboard.get(); } @@ -512,7 +514,7 @@ public synchronized KeyboardServer getKeyboard() { * {@inheritDoc} */ @Override - public synchronized MouseServer getMouse() { + public MouseServer getMouse() { init(); return context.get().mouse.get(); } @@ -521,7 +523,7 @@ public synchronized MouseServer getMouse() { * {@inheritDoc} */ @Override - public synchronized Capabilities getCapabilities() { + public Capabilities getCapabilities() { init(); return context.get().capabilities.get(); } @@ -530,7 +532,7 @@ public synchronized Capabilities getCapabilities() { * {@inheritDoc} */ @Override - public synchronized void close() { + public void close() { init(); context.get().removeItem(); } @@ -539,7 +541,7 @@ public synchronized void close() { * {@inheritDoc} */ @Override - public synchronized String getWindowHandle() { + public String getWindowHandle() { init(); return context.get().itemId(); } @@ -548,7 +550,7 @@ public synchronized String getWindowHandle() { * {@inheritDoc} */ @Override - public synchronized Set getWindowHandles() { + public Set getWindowHandles() { init(); return context.get().itemIds(); } @@ -557,7 +559,7 @@ public synchronized Set getWindowHandles() { * {@inheritDoc} */ @Override - public synchronized OptionsServer manage() { + public OptionsServer manage() { init(); return context.get().options.get(); } @@ -566,7 +568,7 @@ public synchronized OptionsServer manage() { * {@inheritDoc} */ @Override - public synchronized LogsServer logs() { + public LogsServer logs() { return LogsServer.instance(); } @@ -574,7 +576,7 @@ public synchronized LogsServer logs() { * {@inheritDoc} */ @Override - public synchronized NavigationServer navigate() { + public NavigationServer navigate() { init(); return context.get().navigation.get(); } @@ -583,7 +585,7 @@ public synchronized NavigationServer navigate() { * {@inheritDoc} */ @Override - public synchronized void quit() { + public void quit() { getStatusCode(); kill(); } @@ -592,7 +594,7 @@ public synchronized void quit() { * {@inheritDoc} */ @Override - public synchronized TargetLocatorServer switchTo() { + public TargetLocatorServer switchTo() { init(); return context.get().targetLocator.get(); } @@ -601,7 +603,7 @@ public synchronized TargetLocatorServer switchTo() { * {@inheritDoc} */ @Override - public synchronized void kill() { + public void kill() { final ContextItem item = context.get().item(); if (item != null) { item.engine.get().getLoadWorker().cancel(); @@ -615,7 +617,7 @@ public synchronized void kill() { * {@inheritDoc} */ @Override - public synchronized X getScreenshotAs(final OutputType outputType) throws WebDriverException { + public X getScreenshotAs(final OutputType outputType) throws WebDriverException { return outputType.convertFromPngBytes(getScreenshot()); } @@ -623,7 +625,7 @@ public synchronized X getScreenshotAs(final OutputType outputType) throws * {@inheritDoc} */ @Override - public synchronized byte[] getScreenshot() throws WebDriverException { + public byte[] getScreenshot() throws WebDriverException { init(); return context.get().robot.get().screenshot(); } @@ -632,7 +634,7 @@ public synchronized byte[] getScreenshot() throws WebDriverException { * {@inheritDoc} */ @Override - public synchronized File cacheDir() { + public File cacheDir() { return StreamConnection.cacheDir(); } @@ -640,7 +642,7 @@ public synchronized File cacheDir() { * {@inheritDoc} */ @Override - public synchronized File attachmentsDir() { + public File attachmentsDir() { return StreamConnection.attachmentsDir(); } @@ -648,7 +650,7 @@ public synchronized File attachmentsDir() { * {@inheritDoc} */ @Override - public synchronized File mediaDir() { + public File mediaDir() { return StreamConnection.mediaDir(); } } \ No newline at end of file diff --git a/src/com/machinepublishers/jbrowserdriver/PortGroup.java b/src/com/machinepublishers/jbrowserdriver/PortGroup.java new file mode 100644 index 00000000..bdbf0ba6 --- /dev/null +++ b/src/com/machinepublishers/jbrowserdriver/PortGroup.java @@ -0,0 +1,49 @@ +/* + * jBrowserDriver (TM) + * Copyright (C) 2014-2016 Machine Publishers, LLC + * + * Sales and support: ops@machinepublishers.com + * Updates: https://github.com/MachinePublishers/jBrowserDriver + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.machinepublishers.jbrowserdriver; + +import java.io.Serializable; + +class PortGroup implements Serializable { + static final int SIZE = 3; + final long child; + final long parent; + final long parentAlt; + private final String id; + private final int hashCode; + + PortGroup(long child, long parent, long parentAlt) { + this.child = child; + this.parent = parent; + this.parentAlt = parentAlt; + id = new StringBuilder().append(child).append("/").append(parent).append("/").append(parentAlt).toString(); + hashCode = id.hashCode(); + } + + @Override + public int hashCode() { + return hashCode; + } + + @Override + public boolean equals(Object obj) { + return obj instanceof PortGroup && ((PortGroup) obj).id.equals(id); + } +} diff --git a/src/com/machinepublishers/jbrowserdriver/Settings.java b/src/com/machinepublishers/jbrowserdriver/Settings.java index 31915a5d..1ba1cd7b 100644 --- a/src/com/machinepublishers/jbrowserdriver/Settings.java +++ b/src/com/machinepublishers/jbrowserdriver/Settings.java @@ -28,7 +28,9 @@ import java.util.ArrayList; import java.util.Base64; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; +import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -37,7 +39,6 @@ import java.util.logging.Logger; import org.apache.commons.io.output.ByteArrayOutputStream; -import org.apache.commons.lang.StringUtils; import org.openqa.selenium.Capabilities; import org.openqa.selenium.Dimension; import org.openqa.selenium.Platform; @@ -187,7 +188,7 @@ public static class Builder { private File cacheDir; private int cacheEntries = 10 * 1000; private long cacheEntrySize = 1000 * 1000; - private Collection ports = new LinkedHashSet(); + private String portRanges; private int processes = 2 * Runtime.getRuntime().availableProcessors(); private boolean headless = true; private long ajaxWait = 150; @@ -506,13 +507,9 @@ public Builder cacheEntrySize(long bytes) { @Deprecated public Builder ports(int... ports) { System.err.println("jBrowserDriver: The ports setting is deprecated and will be removed in v2.0.0. Use Settings.Builder.processes(..) instead."); - this.ports.clear(); - this.processes = -1; + this.portRanges = null; + this.processes = ports.length; this.host = "127.0.0.1"; - this.ports.add(0); - for (int i = 0; ports != null && i < ports.length; i++) { - this.ports.add(ports[i]); - } return this; } @@ -522,13 +519,9 @@ public Builder ports(int... ports) { @Deprecated public Builder portsMax(int startingPort, int maxProcesses) { System.err.println("jBrowserDriver: The portsMax setting is deprecated and will be removed in v2.0.0. Use Settings.Builder.processes(..) instead."); - this.ports.clear(); - this.processes = -1; + this.portRanges = null; + this.processes = maxProcesses; this.host = "127.0.0.1"; - this.ports.add(0); - for (int i = 0; i < maxProcesses; i++) { - this.ports.add(startingPort + i); - } return this; } @@ -551,7 +544,7 @@ public Builder portsMax(int startingPort, int maxProcesses) { * @return this Builder */ public Builder processes(int maxProcesses) { - this.ports.clear(); + this.portRanges = null; this.processes = maxProcesses; this.host = "127.0.0.1"; return this; @@ -578,7 +571,7 @@ public Builder processes(int maxProcesses) { * @return this Builder */ public Builder processes(int maxProcesses, String host) { - this.ports.clear(); + this.portRanges = null; this.processes = maxProcesses; this.host = host; return this; @@ -586,9 +579,9 @@ public Builder processes(int maxProcesses, String host) { /** * The ports used by {@link JBrowserDriver} instances and the parent process. - * + *

* The max number of instances that can run concurrently is inferred from the number of ports provided - * (which will be one less than the number of ports provided, to account for the port dedicated to the parent process). + * (every instance requires three ports -- e.g., for up to 8 instances you need 24 ports). *

* Each instance of JBrowserDriver is backed by a separate Java process operated via RMI. *

@@ -604,12 +597,11 @@ public Builder processes(int maxProcesses, String host) { * * @param portRanges * A comma separated list of ports and/or port ranges - * (which are inclusive and separated by a dash) -- e.g., 10000-10007,12500,12502,15377-15380 + * (ranges are inclusive and separated by a dash) -- e.g., 10000-10007,12500,12502,15376-15380 * @return this Builder */ public Builder processes(String portRanges) { - this.ports.clear(); - this.ports.addAll(parsePorts(portRanges)); + this.portRanges = portRanges; this.processes = -1; this.host = "127.0.0.1"; return this; @@ -617,9 +609,9 @@ public Builder processes(String portRanges) { /** * The ports and host/IP used by {@link JBrowserDriver} instances and the parent process. - * + *

* The max number of instances that can run concurrently is inferred from the number of ports provided - * (which will be one less than the number of ports provided, to account for the port dedicated to the parent process). + * (every instance requires three ports -- e.g., for up to 8 instances you need 24 ports). *

* Each instance of JBrowserDriver is backed by a separate Java process operated via RMI. *

@@ -635,13 +627,12 @@ public Builder processes(String portRanges) { * * @param portRanges * A comma separated list of ports and/or port ranges - * (which are inclusive and separated by a dash) -- e.g., 10000-10007,12500,12502,15377-15380 + * (ranges are inclusive and separated by a dash) -- e.g., 10000-10007,12500,12502,15376-15380 * @param host * @return this Builder */ public Builder processes(String portRanges, String host) { - this.ports.clear(); - this.ports.addAll(parsePorts(portRanges)); + this.portRanges = portRanges; this.processes = -1; this.host = host; return this; @@ -1126,8 +1117,7 @@ public Capabilities buildCapabilities() { set(capabilities, PropertyName.LOGGER, this.logger); set(capabilities, PropertyName.HEAD_SCRIPT, this.headScript); set(capabilities, PropertyName.HOST, this.host); - final String joinedPorts = StringUtils.join(this.ports, ','); - set(capabilities, PropertyName.PORT_RANGES, joinedPorts == null || joinedPorts.isEmpty() ? null : joinedPorts); + set(capabilities, PropertyName.PORT_RANGES, this.portRanges == null || this.portRanges.isEmpty() ? null : this.portRanges); if (this.processes > -1) { set(capabilities, PropertyName.PROCESSES, this.processes); } @@ -1182,28 +1172,47 @@ Settings build(Capabilities capabilities) { } } - private static List parsePorts(String portString) { - Collection ports = new LinkedHashSet(); + private static long nextAnonPort() { + --curAnonPort; + if (curAnonPort > -1) { + curAnonPort = -1; + } + return curAnonPort; + } + + private static List parsePorts(int processesMax) { + List portGroups = new ArrayList(); + synchronized (curAnonPortLock) { + for (int i = 0; i < processesMax; i++) { + portGroups.add(new PortGroup(nextAnonPort(), nextAnonPort(), nextAnonPort())); + } + } + return Collections.unmodifiableList(portGroups); + } + + private static List parsePorts(String portString) { + Collection ports = new LinkedHashSet(); String[] ranges = portString.split(","); for (int i = 0; i < ranges.length; i++) { String[] bounds = ranges[i].split("-"); - int low = Integer.parseInt(bounds[0]); - int high = bounds.length > 1 ? Integer.parseInt(bounds[1]) : low; - for (int j = low; j <= high; j++) { + long low = Long.parseLong(bounds[0]); + long high = bounds.length > 1 ? Long.parseLong(bounds[1]) : low; + for (long j = low; j <= high; j++) { ports.add(j); } } - return new ArrayList(ports); - } - private static List parseProcesses(String processesMax) { - int max = Integer.parseInt(processesMax); - List ports = new ArrayList(); - int curPort = -1; - for (int i = 0; i < max + 1; i++) { - ports.add(curPort--); + if (ports.size() % 3 != 0) { + throw new IllegalArgumentException("Each process requires three ports (i.e., number of ports must be a multiple of three)."); } - return ports; + + int max = ports.size() / 3; + Iterator iter = ports.iterator(); + List portGroups = new ArrayList(); + for (int i = 0; i < max; i++) { + portGroups.add(new PortGroup(iter.next(), iter.next(), iter.next())); + } + return Collections.unmodifiableList(portGroups); } private static void set(DesiredCapabilities capabilities, PropertyName name, int val) { @@ -1291,6 +1300,8 @@ private static ResponseInterceptor[] parse(Map capabilities, PropertyName name, return fallback; } + private static long curAnonPort; + private static final Object curAnonPortLock = new Object(); private final RequestHeaders requestHeaders; private final int screenWidth; private final int screenHeight; @@ -1305,8 +1316,7 @@ private static ResponseInterceptor[] parse(Map capabilities, PropertyName name, private final File cacheDir; private final int cacheEntries; private final long cacheEntrySize; - private final List childPorts; - private final int parentPort; + private final List portGroups; private final boolean headless; private final long ajaxWait; private final long ajaxResourceTimeout; @@ -1389,26 +1399,28 @@ private Settings(Settings.Builder builder, Map properties) { this.cacheDir = properties.get(PropertyName.CACHE_DIR.propertyName) == null ? builder.cacheDir : new File(properties.get(PropertyName.CACHE_DIR.propertyName).toString()); this.host = parse(properties, PropertyName.HOST, builder.host); + String portRangesTmp = null; + int processesTmp = -1; if (properties.get(PropertyName.PORTS.propertyName) == null && properties.get(PropertyName.PORT_RANGES.propertyName) == null && properties.get(PropertyName.PROCESSES.propertyName) == null) { if (builder.processes > -1) { - this.childPorts = parseProcesses(Integer.toString(builder.processes)); - this.parentPort = childPorts.remove(0); + processesTmp = builder.processes; } else { - this.childPorts = new ArrayList(builder.ports); - this.parentPort = childPorts.remove(0); + portRangesTmp = builder.portRanges; } } else if (properties.get(PropertyName.PORTS.propertyName) != null) { System.err.println("jBrowserDriver: The jbd.ports property is deprecated and will be removed in v2.0.0. Refer to Settings.Builder.processes(..) API documentation."); - this.childPorts = parsePorts(properties.get(PropertyName.PORTS.propertyName).toString()); - this.parentPort = childPorts.remove(0); + portRangesTmp = properties.get(PropertyName.PORTS.propertyName).toString(); } else if (properties.get(PropertyName.PORT_RANGES.propertyName) != null) { - this.childPorts = parsePorts(properties.get(PropertyName.PORT_RANGES.propertyName).toString()); - this.parentPort = childPorts.remove(0); + portRangesTmp = properties.get(PropertyName.PORT_RANGES.propertyName).toString(); + } else { + processesTmp = Integer.parseInt(properties.get(PropertyName.PROCESSES.propertyName).toString()); + } + if (portRangesTmp == null) { + this.portGroups = parsePorts(processesTmp); } else { - this.childPorts = parseProcesses(properties.get(PropertyName.PROCESSES.propertyName).toString()); - this.parentPort = childPorts.remove(0); + this.portGroups = parsePorts(portRangesTmp); } //backwards compatible property name for versions <= 0.9.1 @@ -1549,12 +1561,8 @@ long cacheEntrySize() { return cacheEntrySize; } - Collection childPorts() { - return childPorts; - } - - int parentPort() { - return parentPort; + List portGroups() { + return portGroups; } String host() { diff --git a/src/com/machinepublishers/jbrowserdriver/SocketFactory.java b/src/com/machinepublishers/jbrowserdriver/SocketFactory.java index 19d25ff0..d861bca9 100644 --- a/src/com/machinepublishers/jbrowserdriver/SocketFactory.java +++ b/src/com/machinepublishers/jbrowserdriver/SocketFactory.java @@ -27,15 +27,19 @@ import java.net.Socket; import java.net.UnknownHostException; import java.rmi.server.RMISocketFactory; +import java.util.concurrent.atomic.AtomicReference; class SocketFactory extends RMISocketFactory implements Serializable { + private static final String apiPackage = Util.class.getPackage().getName() + "."; private final InetAddress host; - private final int parentPort; private final int childPort; - private transient Socket clientSocket = new Socket(); + private final int parentPort; + private final int parentAltPort; private final SocketLock lock; + private transient final AtomicReference clientSocket = new AtomicReference(new Socket()); + private transient final AtomicReference clientAltSocket = new AtomicReference(new Socket()); - SocketFactory(String host, int parentPort, int childPort, SocketLock lock) { + SocketFactory(String host, PortGroup ports, SocketLock lock) { InetAddress hostTmp = null; try { hostTmp = InetAddress.getByName(host); @@ -43,8 +47,9 @@ class SocketFactory extends RMISocketFactory implements Serializable { Util.handleException(e); } this.host = hostTmp; - this.parentPort = parentPort; - this.childPort = childPort; + this.childPort = (int) ports.child; + this.parentPort = (int) ports.parent; + this.parentAltPort = (int) ports.parentAlt; this.lock = lock; } @@ -58,32 +63,52 @@ public ServerSocket createServerSocket(int p) throws IOException { @Override public Socket createSocket(String h, int p) throws IOException { - synchronized (lock) { - final int retries = 5; - for (int i = 1; i <= retries; i++) { - Socket prevClientSocket = clientSocket; + if (Thread.holdsLock(lock) || isDriverApi(new Throwable().getStackTrace())) { + return createSocket(clientSocket, parentPort, childPort, true); + } + return createSocket(clientAltSocket, parentAltPort, childPort, false); + } + + private static boolean isDriverApi(StackTraceElement[] elements) { + for (int i = 1; i < elements.length; i++) { + if (elements[i].getClassName().startsWith(apiPackage)) { + return true; + } + } + return false; + } + + private Socket createSocket(AtomicReference socket, + int localPort, int foreignPort, boolean closePrevious) throws IOException { + synchronized (Object.class) { + final int retries = 15; + for (int i = 1, sleep = 2; i <= retries; i++, sleep *= 2) { try { - clientSocket = new Socket(); - clientSocket.setReuseAddress(true); - clientSocket.setTcpNoDelay(true); - clientSocket.setKeepAlive(true); - //TODO for binding, require parent port and daemon port for each process - clientSocket.bind(new InetSocketAddress(host, parentPort)); - clientSocket.connect(new InetSocketAddress(host, childPort)); - return clientSocket; - } catch (IOException e) { - if (i == retries) { - throw e; + if (closePrevious) { + Util.close(socket.get()); } + socket.set(new Socket()); + socket.get().setReuseAddress(true); + socket.get().setTcpNoDelay(true); + socket.get().setKeepAlive(true); + socket.get().bind(new InetSocketAddress(host, localPort)); + socket.get().connect(new InetSocketAddress(host, foreignPort)); + return socket.get(); + } catch (IOException e) { try { - Thread.sleep(50); - } catch (InterruptedException e2) {} - prevClientSocket.close(); - clientSocket.close(); + if (i == retries) { + throw e; + } + try { + Thread.sleep(sleep); + } catch (InterruptedException e2) {} + } finally { + Util.close(socket.get()); + } } } + throw new IOException(); } - throw new IOException(); } @Override diff --git a/src/com/machinepublishers/jbrowserdriver/diagnostics/Test.java b/src/com/machinepublishers/jbrowserdriver/diagnostics/Test.java index 9c3d7224..ef162715 100644 --- a/src/com/machinepublishers/jbrowserdriver/diagnostics/Test.java +++ b/src/com/machinepublishers/jbrowserdriver/diagnostics/Test.java @@ -54,7 +54,7 @@ public class Test { private static final int TEST_PORT_HTTP = Integer.parseInt(System.getProperty("jbd.testporthttp", "9000")); - private static final String TEST_PORTS_RMI = System.getProperty("jbd.testportsrmi", "10000-10001"); + private static final String TEST_PORTS_RMI = System.getProperty("jbd.testportsrmi", "10000-10002"); private List errors = new ArrayList(); private int curTest = 0; private final boolean inlineOutput;