From 6065e5bc31f182a3b9bf08c72c9fdc10672535c2 Mon Sep 17 00:00:00 2001 From: denderlin Date: Tue, 17 Feb 2026 19:31:48 +0100 Subject: [PATCH 1/2] 120574: Fix log rotation issues caused by lockfile fd inheritance and unchecked flock Close inherited lockfile fd in forked worker processes to prevent POSIX flock from being held indefinitely if the daemon exits abnormally. Check the return value of flock with LOCK_NB so that the watch command skips gracefully when the lock is unavailable instead of proceeding without it. Add error handling to the HUP signal handler for log reopening. --- lib/workhorse/daemon.rb | 6 +++++ lib/workhorse/daemon/shell_handler.rb | 33 +++++++++++++++++++++++---- lib/workhorse/worker.rb | 5 +++- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/lib/workhorse/daemon.rb b/lib/workhorse/daemon.rb index 4fb0b14..1d64e77 100644 --- a/lib/workhorse/daemon.rb +++ b/lib/workhorse/daemon.rb @@ -34,6 +34,10 @@ def initialize(id, name, &block) # @private attr_reader :workers + # @return [File, nil] Lockfile handle to close in forked children + # @private + attr_accessor :lockfile + # Creates a new daemon instance. # # @param pidfile [String, nil] Path template for PID files (use %i placeholder for worker ID) @@ -261,6 +265,8 @@ def start_worker(worker) pid = fork do $0 = process_name(worker) + # Close inherited lockfile fd to prevent holding the flock after parent exits + @lockfile&.close # Reopen pipes to prevent #107576 $stdin.reopen File.open(File::NULL, 'r') null_out = File.open File::NULL, 'w' diff --git a/lib/workhorse/daemon/shell_handler.rb b/lib/workhorse/daemon/shell_handler.rb index 80c19af..1a9c283 100644 --- a/lib/workhorse/daemon/shell_handler.rb +++ b/lib/workhorse/daemon/shell_handler.rb @@ -1,5 +1,7 @@ module Workhorse class Daemon::ShellHandler + class LockNotAvailableError < StandardError; end + def self.run(**options, &block) unless ARGV.one? usage @@ -15,27 +17,43 @@ def self.run(**options, &block) case ARGV.first when 'start' lockfile = acquire_lock(lockfile_path, File::LOCK_EX) + daemon.lockfile = lockfile status = daemon.start when 'stop' lockfile = acquire_lock(lockfile_path, File::LOCK_EX) + daemon.lockfile = lockfile status = daemon.stop when 'kill' - lockfile = acquire_lock(lockfile_path, File::LOCK_EX | File::LOCK_NB) - status = daemon.stop(true) + begin + lockfile = acquire_lock(lockfile_path, File::LOCK_EX | File::LOCK_NB) + daemon.lockfile = lockfile + status = daemon.stop(true) + rescue LockNotAvailableError + status = 1 + end when 'status' lockfile = acquire_lock(lockfile_path, File::LOCK_EX) + daemon.lockfile = lockfile status = daemon.status when 'watch' - lockfile = acquire_lock(lockfile_path, File::LOCK_EX | File::LOCK_NB) - status = daemon.watch + begin + lockfile = acquire_lock(lockfile_path, File::LOCK_EX | File::LOCK_NB) + daemon.lockfile = lockfile + status = daemon.watch + rescue LockNotAvailableError + status = 1 + end when 'restart' lockfile = acquire_lock(lockfile_path, File::LOCK_EX) + daemon.lockfile = lockfile status = daemon.restart when 'restart-logging' lockfile = acquire_lock(lockfile_path, File::LOCK_EX) + daemon.lockfile = lockfile status = daemon.restart_logging when 'soft-restart' lockfile = acquire_lock(lockfile_path, File::LOCK_EX) + daemon.lockfile = lockfile status = daemon.soft_restart when 'usage' usage @@ -105,7 +123,12 @@ def self.usage def self.acquire_lock(lockfile_path, flags) if Workhorse.lock_shell_commands lockfile = File.open(lockfile_path, 'a') - lockfile.flock(flags) + result = lockfile.flock(flags) + + if result == false + lockfile.close + fail LockNotAvailableError, 'Could not acquire lock. Is another workhorse command already running?' + end return lockfile end diff --git a/lib/workhorse/worker.rb b/lib/workhorse/worker.rb index 96409fc..5e55392 100644 --- a/lib/workhorse/worker.rb +++ b/lib/workhorse/worker.rb @@ -296,11 +296,14 @@ def current_memory_consumption def trap_log_reopen Signal.trap(LOG_REOPEN_SIGNAL) do Thread.new do - logger.reopen + logger&.reopen if defined?(ActiveRecord::Base) && ActiveRecord::Base.logger && ActiveRecord::Base.logger != logger ActiveRecord::Base.logger.reopen end + rescue Exception => e + log %(Log reopen signal handler error: #{e.message}\n#{e.backtrace.join("\n")}), :error + Workhorse.on_exception.call(e) end.join end end From e848f34863c2ef64cd86d1cd2f081d61e22b951b Mon Sep 17 00:00:00 2001 From: denderlin Date: Tue, 17 Feb 2026 19:37:17 +0100 Subject: [PATCH 2/2] Update changelog --- CHANGELOG.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c3d5f0..2af5fbe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,21 @@ # Workhorse Changelog +## 1.4.1 - 2026-xx-xx + +* Close inherited lockfile fd in forked worker processes. Previously the + lockfile's file descriptor was inherited by children via `fork`, which could + prevent the POSIX `flock` from being released if the daemon process exited + abnormally. + +* Fix `watch` and `kill` commands to actually abort when the lock is + unavailable. Previously the `flock` return value with `LOCK_NB` was not + checked, so the commands would silently proceed without the lock. + +* Add error handling to the `HUP` signal handler for log reopening. Exceptions + from `logger.reopen` are now caught and reported via `on_exception`. + + Sitrox reference: #120574. + ## 1.4.0 - 2026-02-12 * Stable release based on previous RC release.