From 5920de416c01fef42a5b47827059bc0ef789f572 Mon Sep 17 00:00:00 2001 From: Dmitry Vorotilin Date: Thu, 20 Aug 2026 23:21:12 +0300 Subject: [PATCH 1/8] feat: improve process termination and directory cleanup logic - Add `send_signal` method for robust PID and process group signaling. - Implement `remove_directory` with retries and backoff to handle transient errors. --- lib/ferrum/browser/process.rb | 66 ++++++++++++++++++++++++++++------ sig/ferrum/browser/process.rbs | 8 +++++ spec/unit/process_spec.rb | 39 +++++++++++++++++++- 3 files changed, 102 insertions(+), 11 deletions(-) diff --git a/lib/ferrum/browser/process.rb b/lib/ferrum/browser/process.rb index 50b65a11..a541c247 100644 --- a/lib/ferrum/browser/process.rb +++ b/lib/ferrum/browser/process.rb @@ -23,6 +23,8 @@ class Browser class Process KILL_TIMEOUT = 2 WAIT_KILLED = 0.05 + REMOVE_DIR_RETRIES = 5 + REMOVE_DIR_RETRY_DELAY = 0.1 extend Forwardable @@ -54,13 +56,13 @@ def self.process_killer(pid) # Process.kill is unreliable on Windows ::Process.kill("KILL", pid) unless system("taskkill /f /t /pid #{pid} >NUL 2>NUL") else - ::Process.kill("USR1", pid) + send_signal(pid, "TERM") start = Utils::ElapsedTime.monotonic_time while ::Process.wait(pid, ::Process::WNOHANG).nil? sleep(WAIT_KILLED) next unless Utils::ElapsedTime.timeout?(start, KILL_TIMEOUT) - ::Process.kill("KILL", pid) + send_signal(pid, "KILL") ::Process.wait(pid) break end @@ -70,6 +72,25 @@ def self.process_killer(pid) end end + # + # Signals the whole process group Chrome was spawned into (it's spawned + # with `pgroup: true`), so its child processes (renderer, GPU, zygote, + # ...) are cleaned up too instead of being left orphaned. Falls back to + # signaling just the pid directly if there's no such process group to + # signal (e.g. Xvfb, which isn't spawned with `pgroup: true`) or the + # group can't be signaled. + # + # @param [Integer] pid + # @param [String] name + # + # @return [void] + # + def self.send_signal(pid, name) + ::Process.kill(name, -pid) + rescue Errno::EPERM, Errno::ESRCH + ::Process.kill(name, pid) + end + # # Builds a finalizer proc that removes the directory at the given path. # @@ -79,13 +100,38 @@ def self.process_killer(pid) # @return [Proc] # def self.directory_remover(path) - proc { - begin - FileUtils.remove_entry(path) - rescue StandardError - Errno::ENOENT - end - } + proc { remove_directory(path) } + end + + # + # Removes the given directory, retrying with exponential backoff on + # transient errors. Chrome can briefly hold file locks right after + # being killed, so the directory may not be removable on the first + # try; retrying avoids leaking temp directories in that case. + # + # @param [String] path + # Directory to remove. + # @param [Integer] retries + # Maximum number of removal attempts. + # @param [Float] delay + # Base delay, in seconds, before the first retry; doubles on each + # subsequent attempt. + # + # @return [void] + # + def self.remove_directory(path, retries: REMOVE_DIR_RETRIES, delay: REMOVE_DIR_RETRY_DELAY) + retries.times do |attempt| + FileUtils.remove_entry(path) + break + rescue Errno::ENOENT + break + rescue Errno::ENOTEMPTY, Errno::EBUSY, Errno::EACCES, Errno::EPERM => e + raise e if attempt == retries - 1 + + sleep(delay * (2**attempt)) + end + rescue StandardError => e + warn("[Ferrum] Failed to remove user data dir #{path}: #{e.class}: #{e.message}") end attr_reader :host, :port, :ws_url, :pid, :command, @@ -194,7 +240,7 @@ def kill(pid) end def remove_user_data_dir - self.class.directory_remover(@user_data_dir).call + self.class.remove_directory(@user_data_dir) @user_data_dir = nil end diff --git a/sig/ferrum/browser/process.rbs b/sig/ferrum/browser/process.rbs index 6ffa0cc3..c109179f 100644 --- a/sig/ferrum/browser/process.rbs +++ b/sig/ferrum/browser/process.rbs @@ -5,6 +5,10 @@ module Ferrum WAIT_KILLED: ::Float + REMOVE_DIR_RETRIES: ::Integer + + REMOVE_DIR_RETRY_DELAY: ::Float + attr_reader host: String? attr_reader port: ::Integer? @@ -49,8 +53,12 @@ module Ferrum def self.process_killer: (::Integer pid) -> Proc + def self.send_signal: (::Integer pid, ::String name) -> void + def self.directory_remover: (String path) -> Proc + def self.remove_directory: (::String path, ?retries: ::Integer, ?delay: ::Float) -> void + def initialize: (Browser::Options options) -> void def start: () -> void diff --git a/spec/unit/process_spec.rb b/spec/unit/process_spec.rb index 5ad6c435..9b5523f6 100644 --- a/spec/unit/process_spec.rb +++ b/spec/unit/process_spec.rb @@ -14,7 +14,44 @@ subject.send(:start) - expect(Process).to receive(:kill).with("USR1", 5678).ordered + expect(Process).to receive(:kill).with("TERM", -5678).ordered + expect(Process).to receive(:kill).with("KILL", -5678).ordered + + subject.quit + end + + it "falls back to signaling the leader pid when the group can't be signaled" do + allow(Process).to receive(:spawn).and_return(5678) + allow(Process).to receive(:wait).and_return(nil) + allow(Ferrum::Client).to receive(:new).and_return(double.as_null_object) + + allow_any_instance_of(Ferrum::Browser::Process).to receive(:parse_ws_url) + allow_any_instance_of(Ferrum::Browser::Process).to receive(:parse_json_version) + + subject.send(:start) + + allow(Process).to receive(:kill).with("TERM", -5678).and_raise(Errno::EPERM) + expect(Process).to receive(:kill).with("TERM", 5678).ordered + allow(Process).to receive(:kill).with("KILL", -5678).and_raise(Errno::EPERM) + expect(Process).to receive(:kill).with("KILL", 5678).ordered + + subject.quit + end + + it "falls back to signaling the pid directly when it isn't a process group leader" do + # e.g. Xvfb, which is spawned without `pgroup: true` + allow(Process).to receive(:spawn).and_return(5678) + allow(Process).to receive(:wait).and_return(nil) + allow(Ferrum::Client).to receive(:new).and_return(double.as_null_object) + + allow_any_instance_of(Ferrum::Browser::Process).to receive(:parse_ws_url) + allow_any_instance_of(Ferrum::Browser::Process).to receive(:parse_json_version) + + subject.send(:start) + + allow(Process).to receive(:kill).with("TERM", -5678).and_raise(Errno::ESRCH) + expect(Process).to receive(:kill).with("TERM", 5678).ordered + allow(Process).to receive(:kill).with("KILL", -5678).and_raise(Errno::ESRCH) expect(Process).to receive(:kill).with("KILL", 5678).ordered subject.quit From 3dccd671459cd7dc53c570223eacf2ef78f8d3e8 Mon Sep 17 00:00:00 2001 From: Dmitry Vorotilin Date: Fri, 21 Aug 2026 09:23:16 +0300 Subject: [PATCH 2/8] feat: ensure robust termination of process groups when leader exits - Add `process_group_alive?` method to detect the state of a process group. - Enhance process termination logic to handle scenarios where group members ignore TERM signals. - Add tests to verify process group termination behavior. --- lib/ferrum/browser/process.rb | 29 +++++++++++++++++++++++++++-- sig/ferrum/browser/process.rbs | 2 ++ spec/unit/process_spec.rb | 30 ++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/lib/ferrum/browser/process.rb b/lib/ferrum/browser/process.rb index a541c247..e5001846 100644 --- a/lib/ferrum/browser/process.rb +++ b/lib/ferrum/browser/process.rb @@ -58,12 +58,21 @@ def self.process_killer(pid) else send_signal(pid, "TERM") start = Utils::ElapsedTime.monotonic_time - while ::Process.wait(pid, ::Process::WNOHANG).nil? + leader_exited = false + loop do + # The leader (the pid we actually spawned and can #wait on) may + # exit well before the rest of its process group does -- e.g. a + # renderer/zygote that ignores TERM. Reaping the leader must not + # by itself end the loop, or that child is orphaned forever + # since KILL is only ever sent from the timeout branch below. + leader_exited ||= !::Process.wait(pid, ::Process::WNOHANG).nil? + break if leader_exited && !process_group_alive?(pid) + sleep(WAIT_KILLED) next unless Utils::ElapsedTime.timeout?(start, KILL_TIMEOUT) send_signal(pid, "KILL") - ::Process.wait(pid) + ::Process.wait(pid) unless leader_exited break end end @@ -91,6 +100,22 @@ def self.send_signal(pid, name) ::Process.kill(name, pid) end + # + # Checks whether any process in pid's process group is still alive. + # + # @param [Integer] pid + # + # @return [Boolean] + # + def self.process_group_alive?(pid) + ::Process.kill(0, -pid) + true + rescue Errno::ESRCH + false + rescue Errno::EPERM + true + end + # # Builds a finalizer proc that removes the directory at the given path. # diff --git a/sig/ferrum/browser/process.rbs b/sig/ferrum/browser/process.rbs index c109179f..ceb5169d 100644 --- a/sig/ferrum/browser/process.rbs +++ b/sig/ferrum/browser/process.rbs @@ -55,6 +55,8 @@ module Ferrum def self.send_signal: (::Integer pid, ::String name) -> void + def self.process_group_alive?: (::Integer pid) -> bool + def self.directory_remover: (String path) -> Proc def self.remove_directory: (::String path, ?retries: ::Integer, ?delay: ::Float) -> void diff --git a/spec/unit/process_spec.rb b/spec/unit/process_spec.rb index 9b5523f6..b52f31ef 100644 --- a/spec/unit/process_spec.rb +++ b/spec/unit/process_spec.rb @@ -56,6 +56,36 @@ subject.quit end + + it "kills the whole process group, not just the leader, when a group member ignores TERM", if: Ferrum::Utils::Platform.mri? do + child_pid_path = File.join(Dir.tmpdir, "ferrum-process-spec-child-pid-#{Process.pid}") + + script = <<~RUBY + pid = fork do + trap("TERM", "IGNORE") + sleep 30 + end + Process.detach(pid) + File.write(#{child_pid_path.inspect}, pid.to_s) + sleep 30 + RUBY + leader_pid = Process.spawn(RbConfig.ruby, "-e", script, pgroup: true) + + begin + start = Ferrum::Utils::ElapsedTime.monotonic_time + sleep(0.05) until File.size?(child_pid_path) || Ferrum::Utils::ElapsedTime.timeout?(start, 5) + child_pid = File.read(child_pid_path).to_i + + # A leader that dies promptly on TERM must not short-circuit the + # escalation to KILL for the rest of its process group. + Ferrum::Browser::Process.process_killer(leader_pid).call + sleep(0.2) + + expect { Process.kill(0, child_pid) }.to raise_error(Errno::ESRCH) + ensure + File.delete(child_pid_path) if child_pid && File.exist?(child_pid_path) + end + end end context "env variables" do From 08fbb4210663e4dee06261c3aee014351fd87721 Mon Sep 17 00:00:00 2001 From: Dmitry Vorotilin Date: Fri, 21 Aug 2026 09:31:49 +0300 Subject: [PATCH 3/8] chore: update `.rubocop.yml` --- .rubocop.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index d04c8713..e9b9e3f6 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -6,6 +6,10 @@ AllCops: Layout/FirstArrayElementIndentation: EnforcedStyle: consistent +Layout/LineLength: + Exclude: + - spec/**/* + Naming/PredicateMethod: Enabled: false From 07c0e199c9fec642b8d1c031f9f450cee1177aa2 Mon Sep 17 00:00:00 2001 From: Dmitry Vorotilin Date: Fri, 21 Aug 2026 12:23:53 +0300 Subject: [PATCH 4/8] feat: add async process termination and cleanup option - Enhance `#quit` and `#stop` methods to support non-blocking cleanup with `wait: false`. - Introduce `sync_stop` and `async_stop` for handling synchronous and asynchronous logic. --- lib/ferrum/browser.rb | 13 +++++-- lib/ferrum/browser/process.rb | 64 ++++++++++++++++++++++++++++------ sig/ferrum/browser.rbs | 2 +- sig/ferrum/browser/process.rbs | 6 +++- spec/browser_spec.rb | 16 +++++++++ 5 files changed, 86 insertions(+), 15 deletions(-) diff --git a/lib/ferrum/browser.rb b/lib/ferrum/browser.rb index 196f5d65..f537ccae 100644 --- a/lib/ferrum/browser.rb +++ b/lib/ferrum/browser.rb @@ -238,14 +238,23 @@ def restart # # Terminates the browser process and closes the client connection. # - def quit + # @param [Boolean] wait + # Whether to block until the process is confirmed dead and its user + # data directory removed (the default), or return immediately and run + # that cleanup on a background thread instead. See {Process#stop}. + # + # @return [Thread, nil] + # The background cleanup thread when `wait: false`, `nil` otherwise. + # + def quit(wait: true) return unless @client contexts.close_connections @client.close - @process.stop + thread = @process.stop(wait: wait) @client = @process = @contexts = nil + thread end # diff --git a/lib/ferrum/browser/process.rb b/lib/ferrum/browser/process.rb index e5001846..17c2190a 100644 --- a/lib/ferrum/browser/process.rb +++ b/lib/ferrum/browser/process.rb @@ -11,6 +11,7 @@ require "ferrum/browser/command" require "ferrum/utils/elapsed_time" require "ferrum/utils/platform" +require "ferrum/utils/thread" module Ferrum class Browser @@ -219,17 +220,26 @@ def start # Kills the browser process (and Xvfb, if running) and removes the user # data directory. # - # @return [void] - # - def stop - if @pid - kill(@pid) - kill(@xvfb.pid) if @xvfb&.pid - @pid = nil - end - - remove_user_data_dir if @user_data_dir - ObjectSpace.undefine_finalizer(self) + # @param [Boolean] wait + # Whether to block until the process is confirmed dead and its user + # data directory removed (the default), or return immediately and + # run that cleanup on a background thread instead. Killing a + # stubborn process group can block for up to {KILL_TIMEOUT} seconds, + # plus retries removing its directory, which matters when quitting + # many browsers in a hot path. `wait: false` is not used by + # {#restart}, which always waits so the old process is fully gone + # before the new one starts. + # + # @return [Thread, nil] + # The background cleanup thread when `wait: false`; the caller can + # `#join` it if they need cleanup to have finished, e.g. before + # process exit or before reusing a fixed port. `nil` when `wait: + # true`. + # + def stop(wait: true) + return sync_stop if wait + + async_stop end # @@ -260,6 +270,38 @@ def inspect private + def sync_stop + if @pid + kill(@pid) + kill(@xvfb.pid) if @xvfb&.pid + @pid = nil + end + + remove_user_data_dir if @user_data_dir + ObjectSpace.undefine_finalizer(self) + nil + end + + # + # Snapshots what needs killing/removing, clears instance state so the + # object looks stopped right away, and does the actual work on a + # background thread. The finalizer is left in place as a backup until + # that thread finishes, in case the process exits before it does. + # + def async_stop + pid = @pid + xvfb_pid = @xvfb&.pid + user_data_dir = @user_data_dir + @pid = @user_data_dir = nil + + Utils::Thread.spawn(abort_on_exception: false) do + kill(pid) if pid + kill(xvfb_pid) if xvfb_pid + self.class.remove_directory(user_data_dir) if user_data_dir + ObjectSpace.undefine_finalizer(self) + end + end + def kill(pid) self.class.process_killer(pid).call end diff --git a/sig/ferrum/browser.rbs b/sig/ferrum/browser.rbs index 4ffa780c..411b42aa 100644 --- a/sig/ferrum/browser.rbs +++ b/sig/ferrum/browser.rbs @@ -43,7 +43,7 @@ module Ferrum def restart: () -> void - def quit: () -> void + def quit: (?wait: bool) -> Thread? def crash: () -> Hash[String, untyped] diff --git a/sig/ferrum/browser/process.rbs b/sig/ferrum/browser/process.rbs index ceb5169d..618ba714 100644 --- a/sig/ferrum/browser/process.rbs +++ b/sig/ferrum/browser/process.rbs @@ -65,7 +65,7 @@ module Ferrum def start: () -> void - def stop: () -> void + def stop: (?wait: bool) -> Thread? def restart: () -> void @@ -73,6 +73,10 @@ module Ferrum private + def sync_stop: () -> nil + + def async_stop: () -> Thread + def kill: (::Integer pid) -> void def remove_user_data_dir: () -> void diff --git a/spec/browser_spec.rb b/spec/browser_spec.rb index 139ebc55..bf7955c4 100644 --- a/spec/browser_spec.rb +++ b/spec/browser_spec.rb @@ -379,6 +379,22 @@ expect { Process.kill(0, pid) }.to raise_error(Errno::ESRCH) end + + it "returns immediately with wait: false, finishing cleanup once the returned thread is joined", skip: Ferrum::Utils::Platform.windows? do + browser = Ferrum::Browser.new + pid = browser.process.pid + + start = Ferrum::Utils::ElapsedTime.monotonic_time + thread = browser.quit(wait: false) + elapsed = Ferrum::Utils::ElapsedTime.monotonic_time - start + + expect(thread).to be_a(Thread) + expect(elapsed).to be < 1 + + thread.join + + expect { Process.kill(0, pid) }.to raise_error(Errno::ESRCH) + end end describe "#resize" do From 087bcec9edcd359c7f85b546d3142d4a55c21d55 Mon Sep 17 00:00:00 2001 From: Dmitry Vorotilin Date: Fri, 21 Aug 2026 13:08:46 +0300 Subject: [PATCH 5/8] ref: extract process termination and cleanup logic into `Killer` module - Move process termination, signal handling, and directory cleanup methods to `Process::Killer`. - Simplify `Process` class by delegating termination and cleanup to the new module. --- lib/ferrum/browser/process.rb | 149 ++---------------------- lib/ferrum/browser/process/killer.rb | 160 ++++++++++++++++++++++++++ sig/ferrum/browser/process.rbs | 20 ---- sig/ferrum/browser/process/killer.rbs | 27 +++++ spec/unit/process_spec.rb | 2 +- 5 files changed, 200 insertions(+), 158 deletions(-) create mode 100644 lib/ferrum/browser/process/killer.rb create mode 100644 sig/ferrum/browser/process/killer.rbs diff --git a/lib/ferrum/browser/process.rb b/lib/ferrum/browser/process.rb index 17c2190a..df031a53 100644 --- a/lib/ferrum/browser/process.rb +++ b/lib/ferrum/browser/process.rb @@ -8,6 +8,7 @@ require "ferrum/browser/options/base" require "ferrum/browser/options/chrome" require "ferrum/browser/options/firefox" +require "ferrum/browser/process/killer" require "ferrum/browser/command" require "ferrum/utils/elapsed_time" require "ferrum/utils/platform" @@ -22,11 +23,6 @@ class Browser # stopping/restarting the process and cleaning up its user data directory. # class Process - KILL_TIMEOUT = 2 - WAIT_KILLED = 0.05 - REMOVE_DIR_RETRIES = 5 - REMOVE_DIR_RETRY_DELAY = 0.1 - extend Forwardable delegate path: :command @@ -43,123 +39,6 @@ def self.start(*args) new(*args).tap(&:start) end - # - # Builds a finalizer proc that kills the process with the given pid. - # - # @param [Integer] pid - # Process id to kill. - # - # @return [Proc] - # - def self.process_killer(pid) - proc do - if Utils::Platform.windows? - # Process.kill is unreliable on Windows - ::Process.kill("KILL", pid) unless system("taskkill /f /t /pid #{pid} >NUL 2>NUL") - else - send_signal(pid, "TERM") - start = Utils::ElapsedTime.monotonic_time - leader_exited = false - loop do - # The leader (the pid we actually spawned and can #wait on) may - # exit well before the rest of its process group does -- e.g. a - # renderer/zygote that ignores TERM. Reaping the leader must not - # by itself end the loop, or that child is orphaned forever - # since KILL is only ever sent from the timeout branch below. - leader_exited ||= !::Process.wait(pid, ::Process::WNOHANG).nil? - break if leader_exited && !process_group_alive?(pid) - - sleep(WAIT_KILLED) - next unless Utils::ElapsedTime.timeout?(start, KILL_TIMEOUT) - - send_signal(pid, "KILL") - ::Process.wait(pid) unless leader_exited - break - end - end - rescue Errno::ESRCH, Errno::ECHILD - # nop - end - end - - # - # Signals the whole process group Chrome was spawned into (it's spawned - # with `pgroup: true`), so its child processes (renderer, GPU, zygote, - # ...) are cleaned up too instead of being left orphaned. Falls back to - # signaling just the pid directly if there's no such process group to - # signal (e.g. Xvfb, which isn't spawned with `pgroup: true`) or the - # group can't be signaled. - # - # @param [Integer] pid - # @param [String] name - # - # @return [void] - # - def self.send_signal(pid, name) - ::Process.kill(name, -pid) - rescue Errno::EPERM, Errno::ESRCH - ::Process.kill(name, pid) - end - - # - # Checks whether any process in pid's process group is still alive. - # - # @param [Integer] pid - # - # @return [Boolean] - # - def self.process_group_alive?(pid) - ::Process.kill(0, -pid) - true - rescue Errno::ESRCH - false - rescue Errno::EPERM - true - end - - # - # Builds a finalizer proc that removes the directory at the given path. - # - # @param [String] path - # Directory to remove. - # - # @return [Proc] - # - def self.directory_remover(path) - proc { remove_directory(path) } - end - - # - # Removes the given directory, retrying with exponential backoff on - # transient errors. Chrome can briefly hold file locks right after - # being killed, so the directory may not be removable on the first - # try; retrying avoids leaking temp directories in that case. - # - # @param [String] path - # Directory to remove. - # @param [Integer] retries - # Maximum number of removal attempts. - # @param [Float] delay - # Base delay, in seconds, before the first retry; doubles on each - # subsequent attempt. - # - # @return [void] - # - def self.remove_directory(path, retries: REMOVE_DIR_RETRIES, delay: REMOVE_DIR_RETRY_DELAY) - retries.times do |attempt| - FileUtils.remove_entry(path) - break - rescue Errno::ENOENT - break - rescue Errno::ENOTEMPTY, Errno::EBUSY, Errno::EACCES, Errno::EPERM => e - raise e if attempt == retries - 1 - - sleep(delay * (2**attempt)) - end - rescue StandardError => e - warn("[Ferrum] Failed to remove user data dir #{path}: #{e.class}: #{e.message}") - end - attr_reader :host, :port, :ws_url, :pid, :command, :default_user_agent, :browser_version, :protocol_version, :v8_version, :webkit_version, :xvfb @@ -180,7 +59,7 @@ def initialize(options) @env = Hash(options.env) tmpdir = Dir.mktmpdir("ferrum_user_data_dir_") - ObjectSpace.define_finalizer(self, self.class.directory_remover(tmpdir)) + ObjectSpace.define_finalizer(self, Killer.directory_remover(tmpdir)) @user_data_dir = tmpdir @command = Command.build(options, tmpdir) end @@ -202,12 +81,12 @@ def start if @command.xvfb? @xvfb = Xvfb.start(@command.options) - ObjectSpace.define_finalizer(self, self.class.process_killer(@xvfb.pid)) + ObjectSpace.define_finalizer(self, Killer.process_killer(@xvfb.pid)) end env = Hash(@xvfb&.to_env).merge(@env) @pid = ::Process.spawn(env, *@command.to_a, process_options) - ObjectSpace.define_finalizer(self, self.class.process_killer(@pid)) + ObjectSpace.define_finalizer(self, Killer.process_killer(@pid)) parse_ws_url(read_io, @process_timeout) parse_json_version(ws_url) @@ -224,8 +103,8 @@ def start # Whether to block until the process is confirmed dead and its user # data directory removed (the default), or return immediately and # run that cleanup on a background thread instead. Killing a - # stubborn process group can block for up to {KILL_TIMEOUT} seconds, - # plus retries removing its directory, which matters when quitting + # stubborn process group can block for up to {Killer::KILL_TIMEOUT} + # seconds, plus retries removing its directory, which matters when quitting # many browsers in a hot path. `wait: false` is not used by # {#restart}, which always waits so the old process is fully gone # before the new one starts. @@ -272,8 +151,8 @@ def inspect def sync_stop if @pid - kill(@pid) - kill(@xvfb.pid) if @xvfb&.pid + Killer.kill(@pid) + Killer.kill(@xvfb.pid) if @xvfb&.pid @pid = nil end @@ -295,19 +174,15 @@ def async_stop @pid = @user_data_dir = nil Utils::Thread.spawn(abort_on_exception: false) do - kill(pid) if pid - kill(xvfb_pid) if xvfb_pid - self.class.remove_directory(user_data_dir) if user_data_dir + Killer.kill(pid) if pid + Killer.kill(xvfb_pid) if xvfb_pid + Killer.remove_directory(user_data_dir) if user_data_dir ObjectSpace.undefine_finalizer(self) end end - def kill(pid) - self.class.process_killer(pid).call - end - def remove_user_data_dir - self.class.remove_directory(@user_data_dir) + Killer.remove_directory(@user_data_dir) @user_data_dir = nil end diff --git a/lib/ferrum/browser/process/killer.rb b/lib/ferrum/browser/process/killer.rb new file mode 100644 index 00000000..d41b6788 --- /dev/null +++ b/lib/ferrum/browser/process/killer.rb @@ -0,0 +1,160 @@ +# frozen_string_literal: true + +require "fileutils" +require "ferrum/utils/elapsed_time" +require "ferrum/utils/platform" + +module Ferrum + class Browser + class Process + # + # OS-level process termination and user-data-directory removal, kept + # separate from the rest of {Process} (spawning, CDP handshake, ...). + # Both the GC finalizer and the explicit {Process#stop} path need to + # run the exact same kill/cleanup logic on plain pid/path values -- + # never on `self`. A finalizer proc that closes over the object it's + # attached to creates a reference cycle GC can never collect, so the + # finalizer would never run; keeping this logic as free functions + # taking plain arguments sidesteps that entirely. + # + module Killer + module_function + + KILL_TIMEOUT = 2 + WAIT_KILLED = 0.05 + REMOVE_DIR_RETRIES = 5 + REMOVE_DIR_RETRY_DELAY = 0.1 + + # + # Kills the process at the given pid (and its process group, if + # any), escalating from TERM to KILL if it doesn't exit within + # {KILL_TIMEOUT} seconds. + # + # @param [Integer] pid + # + # @return [void] + # + def kill(pid) + if Utils::Platform.windows? + # Process.kill is unreliable on Windows + ::Process.kill("KILL", pid) unless system("taskkill /f /t /pid #{pid} >NUL 2>NUL") + return + end + + send_signal(pid, "TERM") + start = Utils::ElapsedTime.monotonic_time + leader_exited = false + loop do + # The leader (the pid we actually spawned and can #wait on) may + # exit well before the rest of its process group does -- e.g. a + # renderer/zygote that ignores TERM. Reaping the leader must not + # by itself end the loop, or that child is orphaned forever + # since KILL is only ever sent from the timeout branch below. + leader_exited ||= !::Process.wait(pid, ::Process::WNOHANG).nil? + break if leader_exited && !process_group_alive?(pid) + + sleep(WAIT_KILLED) + next unless Utils::ElapsedTime.timeout?(start, KILL_TIMEOUT) + + send_signal(pid, "KILL") + ::Process.wait(pid) unless leader_exited + break + end + rescue Errno::ESRCH, Errno::ECHILD + # nop + end + + # + # Builds a finalizer proc that kills the process with the given pid. + # + # @param [Integer] pid + # Process id to kill. + # + # @return [Proc] + # + def process_killer(pid) + proc { kill(pid) } + end + + # + # Signals the whole process group Chrome was spawned into (it's + # spawned with `pgroup: true`), so its child processes (renderer, + # GPU, zygote, ...) are cleaned up too instead of being left + # orphaned. Falls back to signaling just the pid directly if + # there's no such process group to signal (e.g. Xvfb, which isn't + # spawned with `pgroup: true`) or the group can't be signaled. + # + # @param [Integer] pid + # @param [String] name + # + # @return [void] + # + def send_signal(pid, name) + ::Process.kill(name, -pid) + rescue Errno::EPERM, Errno::ESRCH + ::Process.kill(name, pid) + end + + # + # Checks whether any process in pid's process group is still alive. + # + # @param [Integer] pid + # + # @return [Boolean] + # + def process_group_alive?(pid) + ::Process.kill(0, -pid) + true + rescue Errno::ESRCH + false + rescue Errno::EPERM + true + end + + # + # Removes the given directory, retrying with exponential backoff on + # transient errors. Chrome can briefly hold file locks right after + # being killed, so the directory may not be removable on the first + # try; retrying avoids leaking temp directories in that case. + # + # @param [String] path + # Directory to remove. + # @param [Integer] retries + # Maximum number of removal attempts. + # @param [Float] delay + # Base delay, in seconds, before the first retry; doubles on each + # subsequent attempt. + # + # @return [void] + # + def remove_directory(path, retries: REMOVE_DIR_RETRIES, delay: REMOVE_DIR_RETRY_DELAY) + retries.times do |attempt| + FileUtils.remove_entry(path) + break + rescue Errno::ENOENT + break + rescue Errno::ENOTEMPTY, Errno::EBUSY, Errno::EACCES, Errno::EPERM => e + raise e if attempt == retries - 1 + + sleep(delay * (2**attempt)) + end + rescue StandardError => e + warn("[Ferrum] Failed to remove user data dir #{path}: #{e.class}: #{e.message}") + end + + # + # Builds a finalizer proc that removes the directory at the given + # path. + # + # @param [String] path + # Directory to remove. + # + # @return [Proc] + # + def directory_remover(path) + proc { remove_directory(path) } + end + end + end + end +end diff --git a/sig/ferrum/browser/process.rbs b/sig/ferrum/browser/process.rbs index 618ba714..d18ce238 100644 --- a/sig/ferrum/browser/process.rbs +++ b/sig/ferrum/browser/process.rbs @@ -1,14 +1,6 @@ module Ferrum class Browser class Process - KILL_TIMEOUT: ::Integer - - WAIT_KILLED: ::Float - - REMOVE_DIR_RETRIES: ::Integer - - REMOVE_DIR_RETRY_DELAY: ::Float - attr_reader host: String? attr_reader port: ::Integer? @@ -51,16 +43,6 @@ module Ferrum def self.start: (Browser::Options options) -> Browser::Process - def self.process_killer: (::Integer pid) -> Proc - - def self.send_signal: (::Integer pid, ::String name) -> void - - def self.process_group_alive?: (::Integer pid) -> bool - - def self.directory_remover: (String path) -> Proc - - def self.remove_directory: (::String path, ?retries: ::Integer, ?delay: ::Float) -> void - def initialize: (Browser::Options options) -> void def start: () -> void @@ -77,8 +59,6 @@ module Ferrum def async_stop: () -> Thread - def kill: (::Integer pid) -> void - def remove_user_data_dir: () -> void def parse_ws_url: (IO read_io, ::Integer timeout) -> String? diff --git a/sig/ferrum/browser/process/killer.rbs b/sig/ferrum/browser/process/killer.rbs new file mode 100644 index 00000000..b410913b --- /dev/null +++ b/sig/ferrum/browser/process/killer.rbs @@ -0,0 +1,27 @@ +module Ferrum + class Browser + class Process + module Killer + KILL_TIMEOUT: ::Integer + + WAIT_KILLED: ::Float + + REMOVE_DIR_RETRIES: ::Integer + + REMOVE_DIR_RETRY_DELAY: ::Float + + def self.kill: (::Integer pid) -> void + + def self.process_killer: (::Integer pid) -> Proc + + def self.send_signal: (::Integer pid, ::String name) -> void + + def self.process_group_alive?: (::Integer pid) -> bool + + def self.remove_directory: (::String path, ?retries: ::Integer, ?delay: ::Float) -> void + + def self.directory_remover: (::String path) -> Proc + end + end + end +end diff --git a/spec/unit/process_spec.rb b/spec/unit/process_spec.rb index b52f31ef..5187c66e 100644 --- a/spec/unit/process_spec.rb +++ b/spec/unit/process_spec.rb @@ -78,7 +78,7 @@ # A leader that dies promptly on TERM must not short-circuit the # escalation to KILL for the rest of its process group. - Ferrum::Browser::Process.process_killer(leader_pid).call + Ferrum::Browser::Process::Killer.kill(leader_pid) sleep(0.2) expect { Process.kill(0, child_pid) }.to raise_error(Errno::ESRCH) From e1c4737f13f66ada0cca2d5c9f209a7330dbc1b0 Mon Sep 17 00:00:00 2001 From: Dmitry Vorotilin Date: Fri, 21 Aug 2026 13:23:25 +0300 Subject: [PATCH 6/8] chore: add tests --- spec/browser_spec.rb | 14 +++++ spec/unit/process/killer_spec.rb | 101 +++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+) create mode 100644 spec/unit/process/killer_spec.rb diff --git a/spec/browser_spec.rb b/spec/browser_spec.rb index bf7955c4..a2c4d276 100644 --- a/spec/browser_spec.rb +++ b/spec/browser_spec.rb @@ -397,6 +397,20 @@ end end + describe "#restart" do + it "blocks until the old process is confirmed dead before returning", skip: Ferrum::Utils::Platform.windows? do + browser = Ferrum::Browser.new + old_pid = browser.process.pid + + browser.restart + + expect { Process.kill(0, old_pid) }.to raise_error(Errno::ESRCH) + expect(browser.process.pid).not_to eq(old_pid) + ensure + browser&.quit + end + end + describe "#resize" do it "allows the viewport to be resized" do browser.go_to diff --git a/spec/unit/process/killer_spec.rb b/spec/unit/process/killer_spec.rb new file mode 100644 index 00000000..50e805fb --- /dev/null +++ b/spec/unit/process/killer_spec.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +require "tmpdir" + +describe Ferrum::Browser::Process::Killer do + describe ".remove_directory" do + it "removes an existing directory" do + dir = Dir.mktmpdir + + described_class.remove_directory(dir) + + expect(Dir.exist?(dir)).to be false + end + + it "treats an already-gone directory as success, without warning" do + dir = Dir.mktmpdir + FileUtils.remove_entry(dir) + + expect(described_class).not_to receive(:warn) + expect { described_class.remove_directory(dir) }.not_to raise_error + end + + it "retries a transient error and succeeds once it clears" do + dir = Dir.mktmpdir + attempts = 0 + allow(FileUtils).to receive(:remove_entry).and_wrap_original do |original, path| + attempts += 1 + raise Errno::ENOTEMPTY, path if attempts == 1 + + original.call(path) + end + + described_class.remove_directory(dir, retries: 3, delay: 0.001) + + expect(attempts).to eq(2) + expect(Dir.exist?(dir)).to be false + end + + it "warns and gives up without raising once retries are exhausted" do + dir = Dir.mktmpdir + allow(FileUtils).to receive(:remove_entry).and_raise(Errno::EBUSY) + + expect(described_class).to receive(:warn).with(a_string_matching(/Errno::EBUSY/)) + expect { described_class.remove_directory(dir, retries: 2, delay: 0.001) }.not_to raise_error + ensure + Dir.rmdir(dir) + end + + it "warns instead of raising on an error it doesn't specifically retry" do + dir = Dir.mktmpdir + allow(FileUtils).to receive(:remove_entry).and_raise(ArgumentError, "boom") + + expect(described_class).to receive(:warn).with(a_string_matching(/ArgumentError: boom/)) + expect { described_class.remove_directory(dir) }.not_to raise_error + ensure + Dir.rmdir(dir) + end + end + + describe ".process_group_alive?", if: Ferrum::Utils::Platform.mri? do + it "returns true while the process group has a live member" do + pid = Process.spawn("sleep 30", pgroup: true) + + begin + expect(described_class.process_group_alive?(pid)).to be true + ensure + Process.kill("KILL", -pid) + Process.wait(pid) + end + end + + it "returns false once the process group is gone" do + pid = Process.spawn("true", pgroup: true) + Process.wait(pid) + + expect(described_class.process_group_alive?(pid)).to be false + end + end + + describe ".process_killer" do + it "builds a proc that kills the given pid when called", if: Ferrum::Utils::Platform.mri? do + pid = Process.spawn("sleep 30", pgroup: true) + expect(described_class).to receive(:kill).with(pid) + + described_class.process_killer(pid).call + ensure + Process.kill("KILL", -pid) + Process.wait(pid) + end + end + + describe ".directory_remover" do + it "builds a proc that removes the given directory when called" do + dir = Dir.mktmpdir + + described_class.directory_remover(dir).call + + expect(Dir.exist?(dir)).to be false + end + end +end From e3acd4ff8306671a75ad919cfca272979d42c43e Mon Sep 17 00:00:00 2001 From: Dmitry Vorotilin Date: Fri, 21 Aug 2026 14:53:16 +0300 Subject: [PATCH 7/8] chore: add changelog entry --- CHANGELOG.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 525d1a41..2e080999 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ ## [Unreleased](https://github.com/rubycdp/ferrum/compare/v0.18.0...main) ## ### Added +- `Ferrum::Browser#quit`/`Ferrum::Browser::Process#stop` accept `wait: false` to return immediately and run process + killing and user-data-directory cleanup on a background thread instead of blocking; the call returns the `Thread` + so callers can `#join` it if they need cleanup to have finished, e.g. before process exit or before reusing a + fixed port. Default (`wait: true`) keeps the previous synchronous behavior; `#restart` always waits. ### Changed @@ -9,6 +13,16 @@ A worker's attach or service-worker detach that timed out (e.g. on a loaded CI runner) would escape unrescued into `Client::Subscriber`'s dispatch thread and kill it, silently breaking further `Target.*` event delivery for the rest of that browser's life, raising `Ferrum::NoSuchTargetError` +- `Ferrum::Browser::Process` killed the browser's leader pid alone with `SIGUSR1`, a signal Chromium has no shutdown + handler for; it now sends `SIGTERM`, escalating to `SIGKILL`, to the whole process group, so renderer/GPU/zygote + child processes are no longer orphaned after `#quit`. A leader process that exited promptly on `TERM` used to + short-circuit escalation to `KILL` for the rest of its process group, so a child that ignored `TERM` + (e.g. a stuck renderer) was left running forever; termination now keeps polling the group until it's actually empty + or the timeout fires. +- Removing the user data directory after `#quit` silently gave up on any error, potentially leaking the temp + directory forever with no indication; it now retries with exponential backoff on transient errors + (`Errno::ENOTEMPTY`/`EBUSY`/`EACCES`/`EPERM`, since Chrome can briefly hold file locks right after being killed) + and warns if it still can't be removed. ### Removed From bec0bedad7a825f061c69b67f9296df8e1b65c36 Mon Sep 17 00:00:00 2001 From: Dmitry Vorotilin Date: Fri, 21 Aug 2026 19:25:47 +0300 Subject: [PATCH 8/8] chore: add docs entry --- docs/1-introduction.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/1-introduction.md b/docs/1-introduction.md index 2940a061..e807db03 100644 --- a/docs/1-introduction.md +++ b/docs/1-introduction.md @@ -118,6 +118,18 @@ browser.reset browser.quit ``` +`#quit` blocks by default until the browser process is confirmed dead and its user data directory removed. Killing a +stubborn process (one that ignores `TERM` and needs `KILL`) can take a couple of seconds, which matters if you're +quitting many browsers in a hot path. Pass `wait: false` to return immediately and run that cleanup on a background +thread instead: + +```ruby +browser = Ferrum::Browser.new +thread = browser.quit(wait: false) +# ... do other work while the browser is killed and its directory removed in the background ... +thread.join # only needed if you must wait for cleanup to finish, e.g. before reusing a fixed port +``` + ## Thread safety Ferrum is fully thread-safe. You can create one browser or a few as you wish and