diff --git a/rb/Steepfile b/rb/Steepfile index f0f3b77bd2ec2..a6556749df561 100644 --- a/rb/Steepfile +++ b/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 diff --git a/rb/lib/selenium/webdriver/common.rb b/rb/lib/selenium/webdriver/common.rb index 0412195a74266..96f125cec0e8d 100644 --- a/rb/lib/selenium/webdriver/common.rb +++ b/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' diff --git a/rb/lib/selenium/webdriver/common/port_lock.rb b/rb/lib/selenium/webdriver/common/port_lock.rb new file mode 100644 index 0000000000000..4debdf962f011 --- /dev/null +++ b/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 diff --git a/rb/lib/selenium/webdriver/common/service_manager.rb b/rb/lib/selenium/webdriver/common/service_manager.rb index 45fbf0b5011da..01044470b579f 100644 --- a/rb/lib/selenium/webdriver/common/service_manager.rb +++ b/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 diff --git a/rb/lib/selenium/webdriver/common/socket_lock.rb b/rb/lib/selenium/webdriver/common/socket_lock.rb deleted file mode 100644 index e3189b92eae12..0000000000000 --- a/rb/lib/selenium/webdriver/common/socket_lock.rb +++ /dev/null @@ -1,82 +0,0 @@ -# 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. - -module Selenium - module WebDriver - # - # @api private - # - - class SocketLock - def initialize(port, timeout) - @port = port - @server = nil - @timeout = timeout - end - - # - # Attempt to acquire a lock on the given port. Control is yielded to an - # execution block if the lock could be successfully obtained. - # - - def locked - lock - - begin - yield - ensure - release - end - end - - private - - def lock - max_time = current_time + @timeout - - sleep 0.1 until can_lock? || current_time >= max_time - - return if did_lock? - - raise Error::WebDriverError, "unable to bind to locking port #{@port} within #{@timeout} seconds" - end - - def current_time - Process.clock_gettime(Process::CLOCK_MONOTONIC) - end - - def release - @server&.close - end - - def can_lock? - @server = TCPServer.new(Platform.localhost, @port) - @server.close_on_exec = true - true - rescue SocketError, Errno::EADDRINUSE, Errno::EBADF => e - WebDriver.logger.debug("#{self}: #{e.message}", id: :driver_service) - false - end - - def did_lock? - !@server.nil? - end - end # SocketLock - end # WebDriver -end # Selenium diff --git a/rb/sig/lib/selenium/webdriver/common/socket_lock.rbs b/rb/sig/lib/selenium/webdriver/common/port_lock.rbs similarity index 83% rename from rb/sig/lib/selenium/webdriver/common/socket_lock.rbs rename to rb/sig/lib/selenium/webdriver/common/port_lock.rbs index 07d5989451bcc..e9e9a1ad7c592 100644 --- a/rb/sig/lib/selenium/webdriver/common/socket_lock.rbs +++ b/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 diff --git a/rb/sig/lib/selenium/webdriver/common/service_manager.rbs b/rb/sig/lib/selenium/webdriver/common/service_manager.rbs index 503e27c7aea4a..082426a31fe21 100644 --- a/rb/sig/lib/selenium/webdriver/common/service_manager.rbs +++ b/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 diff --git a/rb/spec/unit/selenium/webdriver/common/port_lock_spec.rb b/rb/spec/unit/selenium/webdriver/common/port_lock_spec.rb new file mode 100644 index 0000000000000..b6b8bb2dc76ef --- /dev/null +++ b/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