[rb] Lock the starting port in a file rather than on the port below it

rb/Steepfile

@@ -49,8 +49,6 @@ target :lib do
     'lib/selenium/webdriver/common/child_process.rb',
     # Ignore due to Net::HTTP not being found on line 49
     'lib/selenium/webdriver/chromium/driver.rb',
-    # Ignore due to positional argument error with TCPServer rescue on line 69
-    'lib/selenium/webdriver/common/socket_lock.rb',
     # Ignore due to is_a? bot error on line 70
     'lib/selenium/webdriver/remote/driver.rb',
     # Ignore due to line 118 causing an error with URI & Net::HTTP

rb/lib/selenium/webdriver/common.rb

@@ -28,7 +28,7 @@
 require 'selenium/webdriver/common/selenium_manager'
 require 'selenium/webdriver/common/service'
 require 'selenium/webdriver/common/service_manager'
-require 'selenium/webdriver/common/socket_lock'
+require 'selenium/webdriver/common/port_lock'
 require 'selenium/webdriver/common/socket_poller'
 require 'selenium/webdriver/common/port_prober'
 require 'selenium/webdriver/common/zipper'

rb/lib/selenium/webdriver/common/port_lock.rb

@@ -0,0 +1,95 @@
+# frozen_string_literal: true
+
+# Licensed to the Software Freedom Conservancy (SFC) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The SFC licenses this file
+# to you 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.
+
+require 'tmpdir'
+
+module Selenium
+  module WebDriver
+    #
+    # Holds a lock on a starting port so that two processes probing for a free port
+    # cannot both claim it. The lock lives in a file rather than on a TCP port, so it
+    # needs no port of its own and is released even if the process is killed.
+    #
+    # @api private
+    #
+
+    class PortLock
+      def initialize(port, timeout)
+        @path = File.join(Dir.tmpdir, "selenium-port-#{port}.lock")
+        @timeout = timeout
+      end
+
+      #
+      # Attempt to acquire the lock. Control is yielded to an execution block once it
+      # is held, and the lock is released when the block finishes.
+      #
+
+      def locked
+        file = lock
+
+        begin
+          yield
+        ensure
+          release(file)
+        end
+      end
+
+      private
+
+      def lock
+        max_time = current_time + @timeout
+
+        loop do
+          file = open_lock_file
+          return file if file&.flock(File::LOCK_EX | File::LOCK_NB)
+
+          file&.close
+          break if current_time >= max_time
+
+          sleep 0.1
+        end
+
+        raise Error::WebDriverError, "unable to acquire #{@path} within #{@timeout} seconds"
+      end
+
+      # nil means the lock is not available yet: Windows refuses to open a file another
+      # process has locked. The handle outlives this method when it is returned, since it
+      # holds the lock until #locked closes it.
+      def open_lock_file
+        file = File.open(@path, File::RDWR | File::CREAT, 0o600) # rubocop:disable Style/FileOpen
+        file.close_on_exec = true
+        file
+      rescue Errno::EROFS => e
+        raise Error::WebDriverError, "unable to create the lock file #{@path}: #{e.message}"
+      rescue Errno::EACCES => e
+        WebDriver.logger.debug("#{self}: #{e.message}", id: :driver_service)
+        nil
+      end
+
+      def release(file)
+        file.flock(File::LOCK_UN)
+        file.close
+      end
+
+      def current_time
+        Process.clock_gettime(Process::CLOCK_MONOTONIC)
+      end
+    end # PortLock
+  end # WebDriver
+end # Selenium

rb/lib/selenium/webdriver/common/service_manager.rb

@@ -27,7 +27,7 @@ module WebDriver
     #
     class ServiceManager
       START_TIMEOUT = 20
-      SOCKET_LOCK_TIMEOUT = 45
+      PORT_LOCK_TIMEOUT = 45
       STOP_TIMEOUT = 20
 
       #
@@ -52,7 +52,7 @@ def start
 
         Platform.exit_hook { stop } # make sure we don't leave the server running
 
-        socket_lock.locked do
+        port_lock.locked do
           find_free_port
           start_process
           connect_until_stable
@@ -169,8 +169,8 @@ def cannot_connect_error_text
         "unable to connect to #{@executable_path} #{@host}:#{@port}"
       end
 
-      def socket_lock
-        @socket_lock ||= SocketLock.new(@port - 1, SOCKET_LOCK_TIMEOUT)
+      def port_lock
+        @port_lock ||= PortLock.new(@port, PORT_LOCK_TIMEOUT)
       end
     end # Service
   end # WebDriver

rb/sig/lib/selenium/webdriver/common/socket_lock.rbs → rb/sig/lib/selenium/webdriver/common/port_lock.rbs

@@ -18,10 +18,8 @@
 
 module Selenium
   module WebDriver
-    class SocketLock
-      @port: untyped
-
-      @server: untyped
+    class PortLock
+      @path: untyped
 
       @timeout: untyped
 
@@ -31,15 +29,13 @@ module Selenium
 
       private
 
-      def lock: () -> untyped?
-
-      def current_time: () -> untyped
+      def lock: () -> untyped
 
-      def release: () -> untyped
+      def open_lock_file: () -> untyped
 
-      def can_lock?: () -> untyped
+      def release: (untyped file) -> untyped
 
-      def did_lock?: () -> untyped
+      def current_time: () -> untyped
     end
   end
 end

rb/sig/lib/selenium/webdriver/common/service_manager.rbs

@@ -35,11 +35,11 @@ module Selenium
 
       @process: untyped
 
-      @socket_lock: untyped
+      @port_lock: untyped
 
       START_TIMEOUT: Integer
 
-      SOCKET_LOCK_TIMEOUT: Integer
+      PORT_LOCK_TIMEOUT: Integer
 
       STOP_TIMEOUT: Integer
 
@@ -77,7 +77,7 @@ module Selenium
 
       def cannot_connect_error_text: () -> String
 
-      def socket_lock: () -> untyped
+      def port_lock: () -> untyped
     end
   end
 end

rb/spec/unit/selenium/webdriver/common/port_lock_spec.rb

@@ -0,0 +1,77 @@
+# frozen_string_literal: true
+
+# Licensed to the Software Freedom Conservancy (SFC) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The SFC licenses this file
+# to you 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.
+
+require File.expand_path('../spec_helper', __dir__)
+
+module Selenium
+  module WebDriver
+    describe PortLock do
+      subject(:port_lock) { described_class.new(port, 2) }
+
+      # The lock file is named after the port, so a fixed one would collide with a real
+      # driver on 4444 and with any other run of this spec on the same machine.
+      let(:port) { 40_000 + (Process.pid % 10_000) }
+
+      it 'yields to the block' do
+        expect { |block| port_lock.locked(&block) }.to yield_control
+      end
+
+      it 'returns what the block returned' do
+        expect(port_lock.locked { :started }).to be(:started)
+      end
+
+      it 'releases the lock once the block is done' do
+        port_lock.locked { :first }
+
+        expect(described_class.new(port, 2).locked { :second }).to be(:second)
+      end
+
+      it 'releases the lock when the block raises' do
+        expect { port_lock.locked { raise 'boom' } }.to raise_error('boom')
+        expect(described_class.new(port, 2).locked { :second }).to be(:second)
+      end
+
+      it 'ignores a neighbouring port being in use' do
+        neighbour = TCPServer.new(Platform.localhost, 0)
+        busy = described_class.new(neighbour.addr[1], 2)
+
+        expect(busy.locked { :started }).to be(:started)
+      ensure
+        neighbour&.close
+      end
+
+      it 'keeps a second lock on the same port out' do
+        expect {
+          port_lock.locked { described_class.new(port, 0).locked { :never } }
+        }.to raise_error(Error::WebDriverError, /unable to acquire/)
+      end
+
+      it 'lets a lock on a different port through' do
+        expect(port_lock.locked { described_class.new(port + 1, 0).locked { :other } }).to be(:other)
+      end
+
+      it 'fails without waiting out the timeout when the lock file cannot be created' do
+        allow(File).to receive(:open).and_raise(Errno::EROFS)
+
+        expect { port_lock.locked { :never } }
+          .to raise_error(Error::WebDriverError, /unable to create the lock file/)
+      end
+    end
+  end
+end