rb: Use socket lock when starting PhantomJS driver

This extends 437725c to PhantomJS because any external process starting
with specific port suffers the same race condition.

Author: p0deje

rb/lib/selenium/webdriver/chrome/bridge.rb

@@ -103,7 +103,6 @@ def create_capabilities(opts)
                                   'extensions' => data['extensions']
           end
 
-
           chrome_options['binary']                   = Chrome.path if Chrome.path
           chrome_options['nativeEvents']             = true if native_events
           chrome_options['verbose']                  = true if verbose

rb/lib/selenium/webdriver/chrome/service.rb

@@ -24,6 +24,7 @@ module Chrome
       #
       # @api private
       #
+
       class Service
         START_TIMEOUT       = 20
         SOCKET_LOCK_TIMEOUT = 45
@@ -90,6 +91,8 @@ def uri
           URI.parse "http://#{@host}:#{@port}"
         end
 
+        private
+
         def find_free_port
           @port = PortProber.above @port
         end
@@ -103,9 +106,9 @@ def start_process
         end
 
         def connect_until_stable
-          @socket_poller = SocketPoller.new @host, @port, START_TIMEOUT
+          socket_poller = SocketPoller.new @host, @port, START_TIMEOUT
 
-          unless @socket_poller.connected?
+          unless socket_poller.connected?
             raise Error::WebDriverError, "unable to connect to chromedriver #{@host}:#{@port}"
           end
         end

rb/lib/selenium/webdriver/common/socket_lock.rb

@@ -19,6 +19,11 @@
 
 module Selenium
   module WebDriver
+
+    #
+    # @api private
+    #
+
     class SocketLock
 
       def initialize(port, timeout)

rb/lib/selenium/webdriver/edge/service.rb

@@ -24,6 +24,7 @@ module Edge
       #
       # @api private
       #
+
       class Service
         START_TIMEOUT       = 20
         SOCKET_LOCK_TIMEOUT = 45
@@ -103,9 +104,9 @@ def start_process
         end
 
         def connect_until_stable
-          @socket_poller = SocketPoller.new @host, @port, START_TIMEOUT
+          socket_poller = SocketPoller.new @host, @port, START_TIMEOUT
 
-          unless @socket_poller.connected?
+          unless socket_poller.connected?
             raise Error::WebDriverError, "unable to connect to MicrosoftWebDriver #{@host}:#{@port}"
           end
         end

rb/lib/selenium/webdriver/phantomjs/service.rb

@@ -26,12 +26,11 @@ module PhantomJS
       #
 
       class Service
-        START_TIMEOUT = 20
-        STOP_TIMEOUT  = 5
-        DEFAULT_PORT  = 8910
-        MISSING_TEXT  = "Unable to find phantomjs executable."
-
-        attr_reader :uri
+        START_TIMEOUT       = 20
+        SOCKET_LOCK_TIMEOUT = 45
+        STOP_TIMEOUT        = 5
+        DEFAULT_PORT        = 8910
+        MISSING_TEXT        = "Unable to find phantomjs executable."
 
         def self.executable_path
           @executable_path ||= (
@@ -44,35 +43,32 @@ def self.executable_path
         end
 
         def self.default_service(port = nil)
-          new executable_path, port || PortProber.above(DEFAULT_PORT)
+          new executable_path, DEFAULT_PORT
         end
 
         def initialize(executable_path, port)
-          @uri        = URI.parse "http://#{Platform.localhost}:#{port}"
+          @host       = Platform.localhost
           @executable = executable_path
         end
 
         def start(args = [])
           if @process && @process.alive?
-            raise "already started: #{@uri.inspect} #{@executable.inspect}"
+            raise "already started: #{uri.inspect} #{@executable.inspect}"
           end
 
-          @process = create_process(args)
-          @process.start
-
-          socket_poller = SocketPoller.new Platform.localhost, @uri.port, START_TIMEOUT
+          Platform.exit_hook { stop } # make sure we don't leave the server running
 
-          unless socket_poller.connected?
-            raise Error::WebDriverError, "unable to connect to phantomjs @ #{@uri} after #{START_TIMEOUT} seconds"
+          socket_lock.locked do
+            find_free_port
+            start_process(args)
+            connect_until_stable
           end
-
-          Platform.exit_hook { stop } # make sure we don't leave the server running
         end
 
         def stop
           return if @process.nil? || @process.exited?
 
-          Net::HTTP.start(uri.host, uri.port) do |http|
+          Net::HTTP.start(@host, @port) do |http|
             http.open_timeout = STOP_TIMEOUT / 2
             http.read_timeout = STOP_TIMEOUT / 2
 
@@ -89,9 +85,19 @@ def stop
           end
         end
 
-        def create_process(args)
-          server_command = [@executable, "--webdriver=#{@uri.port}", *args]
-          process = ChildProcess.build(*server_command.compact)
+        def find_free_port
+          @port = PortProber.above @port
+        end
+
+        def uri
+          URI.parse "http://#{@host}:#{@port}"
+        end
+
+        private
+
+        def start_process(args)
+          server_command = [@executable, "--webdriver=#{@port}", *args]
+          @process = ChildProcess.build(*server_command.compact)
 
           if $DEBUG == true
             process.io.inherit!
@@ -100,10 +106,22 @@ def create_process(args)
             process.io.stdout = process.io.stderr = File.new(Platform.null_device, 'w')
           end
 
-          process
+          @process.start
+        end
+
+        def connect_until_stable
+          socket_poller = SocketPoller.new @host, @port, START_TIMEOUT
+
+          unless socket_poller.connected?
+            raise Error::WebDriverError, "unable to connect to phantomjs @ #{uri} after #{START_TIMEOUT} seconds"
+          end
+        end
+
+        def socket_lock
+          @socket_lock ||= SocketLock.new(@port - 1, SOCKET_LOCK_TIMEOUT)
         end
 
       end # Service
     end # PhantomJS
   end # WebDriver
-end # Service
+end # Service

rb/spec/unit/selenium/webdriver/chrome/service_spec.rb

@@ -41,7 +41,7 @@ module Chrome
             mock_process
           end
 
-          Service.default_service.start_process
+          Service.default_service.send(:start_process)
         end
 
         it "finds the Chrome server binary by searching PATH" do